[PATCH weston 2/2] main: don't leak option strings

[Ryo Munakata]

On Mon, 8 Sep 2014 09:28:17 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Sun, 07 Sep 2014 21:30:52 -0700
> Bill Spitzak <spitzak at gmail.com> wrote:
> 
> > On 09/05/2014 10:47 PM, Ryo Munakata wrote:
> > >
> > > Hi, Bill.
> > >
> > > Thank you for reviewing.
> > >
> > > On Fri, 05 Sep 2014 18:08:44 -0700
> > > Bill Spitzak <spitzak at gmail.com> wrote:
> > >
> > >> I think if the strdup's were removed it would get rid of the memory leak
> > >> in a much cleaner way.
> > >
> > > Do you mean the strdup's in handle_options()?
> > >
> > > handle_options() copies the content of argv, so if handle_option() doesn't copy it and someone overwrites it, we can't get correct option strings later.
> > > argv is not immutable and this is why handle_options() copies the content of argv, I think.
> > 
> > I think it is safe to assume the contents of argv will not get overwritten.
> > 
> > It is common to copy argv itself and modify that, but not to modify the 
> > strings it points at.
> 
> This would be feasible only if we can guarantee that no-one will ever
> need to store a dynamically allocated string to any of the assigned
> variables. If something produces a dynamically allocated string, then
> you would need to track if you can free() the string or not.
> 
> So to keep things plain and simple, we just choose to dynamically
> allocate all strings, so that free() must be done unconditionally. It
> is also future-proof in case anyone needs to add a thing that sets a
> dynamically allocated string, like something that is a combination of
> an environment variable and something else.
> 
> Let's keep all the dynamic allocations in place, because changing it
> would affect all users of the command line and config file parsers.
> 
> 
> Thanks,
> pq

I agree.
Let's go with that.

Thanks.
-- 
Ryo Munakata <ryomnktml at gmail.com>