Re: Memleak in glob()

[Bruno Haible]

Hi Tim,

> > Did you find
> > volunteers for the triage of the "defects" on scan.coverity.com? I stopped
> > looking into it when you said you would find some people.
> 
> Sadly, I was too optimistic. I didn't forget about, and working on those is 
> still on my list. But currently I am overwhelmed with stuff that has higher 
> priority.

OK, this means the "find some people" is not going to happen soon. That I'll
continue the review on scan.coverity.com (as I don't find it _that_ low
priority, especially when I can concentrate on specific categories of issues).

> You could set up a cron job to automatically upload 
> to Coverity, e.g. once a week or once a day. You can find instructions one 
> the 
> 'upload for file analysis' page.

Good idea. I haven't found a 'upload for file analysis' page, but the
instructions (including the mandatory, project specific token) are on the
project's "Upload a Project Build" page. In progress...

> > The 'return 0;' in line 1104 is correct, because 'dirname' is stuffed into
> > the result array in the lines before. Therefore dirname must NOT be freed
> > here.
> 
> This is right for one path, but not all the paths before 'return 0' stuff 
> 'dirname' into that array.

Ah, indeed. We should really rerun coverity after making changes to glob.c.

> In code that I am responsible for, 
> especially complex code as glob(), I also set pointers to NULL after free() 
> or 
> when assigning/moving to another variable.

Such a coding style does not help. What you need is to reduce the complexity,
either by introducing inline functions, or by at least reducing the scope
of the variables in the big function.

The only situations where "set pointers to NULL after free()" is useful are:
  - when you're dealing with global variables or heap-allocated struct members
    (or class members in C++), or
  - when there is a (conservative) garbage collector around.

Bruno