On 08/20/2013 03:40 PM, 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. >
Yes, indeed. Thanks for reviewing.
Stepping two steps back, I am trying to figure out, what this repo_abrev
thing is doing, as I could find no documentation.
It's passed as a double pointer as declared in mailmap.h:
int read_mailmap(struct string_list *map, char **repo_abbrev);
However grepping for "read_mailmap(" (bracket to prevent finding
read_mailmap_XXX as often used in mailmap.c itself)
grep -nHIirF --exclude-dir=.git -- "read_mailmap("
throughout all the sources I just find one occurence having the
second argument not being 'NULL' and that is in
builtin/shortlog.c:212: read_mailmap(&log->mailmap,
&log->common_repo_prefix);
which turns out to be:
void shortlog_init(struct shortlog *log)
{
memset(log, 0, sizeof(*log));
read_mailmap(&log->mailmap, &log->common_repo_prefix);
...
So we're passing there an address, which was just set to zero.
This is the only occurence of passing a value at all and the value
being passed is 0, so the free in the original patch doesn't need
that check either.
As I am resending the patch, could somebody please explain me
the mechanism of the "# repo-abbrev:" line? Even git itself doesn't
use it in the .mailmap file, but a quick google search shows up only
kernel repositories.
Stefan
signature.asc
Description: OpenPGP digital signature
