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

Pekka Paalanen ppaalanen at gmail.com
Sun Sep 7 23:28:17 PDT 2014


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


More information about the wayland-devel mailing list