On Tue, Aug 20, 2013 at 03:40:02PM +0200, Thomas Rast wrote:

> Stefan Beller <stefanbel...@googlemail.com> writes:
> > The condition as it is written in that line was most likely intended to
> > check for the pointer passed to free(), rather than checking for the
> > 'repo_abbrev', which is already checked against being non null at the
> > beginning of the function.
> [...]
> > -                   if (repo_abbrev)
> > +                   if (*repo_abbrev)
> >                             free(*repo_abbrev);
> But now the test is useless, because free(NULL) is defined to be a
> no-op.

Yeah, I think we should just drop the conditional completely.

I am not sure of the complete back-story. The earlier check for
repo_abbrev to be non-NULL was added by 8503ee4, after this check on
free() already existed. So that was when this conditional became

But the line right after this one unconditionally assigns to
"*repo_abbrev", so we would always segfault in such a case anyway (which
is what 8503ee4 was fixing).

So I think it was either a misguided "don't pass NULL to free" check
that was simply wrong, or it was an incomplete "make sure repo_abbrev is
not NULL" check. And the first is useless, and the second is now
redundant due to 8503ee4. So it should simply be free().

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to