On Tue, Aug 20, 2013 at 03:40:02PM +0200, Thomas Rast wrote: > Stefan Beller <stefanbeller@xxxxxxxxxxxxxx> 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 redundant. 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(). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html