"David Rydh" <dary@xxxxxxxxxxxxxxxxx> writes:
> Commit b8f26269 (fix directory separator treatment on Windows,
> 30-06-2009) introduced a bug on Mac OS X. The problem is that
> basename() may return a pointer to internal static storage space that
> will be overwritten by subsequent calls:
>
>> git mv dir/a.txt dir/b.txt other/
>
> Checking rename of 'dir/a.txt' to 'other/b.txt'
> Checking rename of 'dir/b.txt' to 'other/b.txt'
Good spotting. basename(3)/dirname(3) are tricky functions to use
correctly. In addition to the "static storage" implementation and its
associated problem you encountered, they can also be implemented to modify
the given string in place, and can even return a pointer _into_ the given
string, which means that if you do:
duped = strdup(string);
duped = basename(duped);
... use duped ...
free(duped);
you may be burned quite badly. The other call site of basename(3) is in
diff.c and it looks safe.
> diff --git a/setup.c b/setup.c
> index 710e2f3..80cf535 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -132,8 +132,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
> return NULL;
>
> if (!entry) {
> - static const char *spec[2];
> - spec[0] = prefix;
> + const char **spec = xmalloc(sizeof(char *) * 2);
> + spec[0] = xstrdup(prefix);
> spec[1] = NULL;
> return spec;
> }
I don't understand this change. Because elements of returned pathspec
from this function are often simply the pathspec argument itself (which in
turn is often argv[] of the calling program), and other times allocated by
this function, the callers are never going to free() them.
--
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