Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes:
> The git_snpath() and git_pathdup() functions both use the (static)
> function vsnpath() in their implementation. Also, they both discard
> the return value of vsnpath(), which has the effect of ignoring the
> side effect of calling cleanup_path() in the non-error return path.
>
> In order to ensure that the required cleanup happens, we use the
> pointer returned by vsnpath(), rather than the buffer passed into
> vsnpath(), to derive the return value from git_snpath() and
> git_pathdup().
>
> Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx>
> ---
> path.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/path.c b/path.c
> index 9eb5333..741ae77 100644
> --- a/path.c
> +++ b/path.c
> @@ -70,21 +70,22 @@ bad:
>
> char *git_snpath(char *buf, size_t n, const char *fmt, ...)
> {
> + char *ret;
> va_list args;
> va_start(args, fmt);
> - (void)vsnpath(buf, n, fmt, args);
> + ret = vsnpath(buf, n, fmt, args);
> va_end(args);
> - return buf;
> + return ret;
> }
>
> char *git_pathdup(const char *fmt, ...)
> {
> - char path[PATH_MAX];
> + char path[PATH_MAX], *ret;
> va_list args;
> va_start(args, fmt);
> - (void)vsnpath(path, sizeof(path), fmt, args);
> + ret = vsnpath(path, sizeof(path), fmt, args);
> va_end(args);
> - return xstrdup(path);
> + return xstrdup(ret);
> }
>
> char *mkpathdup(const char *fmt, ...)
cleanup_path() is a quick-and-dirty hack that only deals with
leading ".///" (e.g. "foo//bar" is not reduced to "foo/bar"), and
the callers that allocate 4 bytes for buf to hold "foo" may not be
able to fit it if for some reason there are some bytes that must be
cleaned, e.g. ".///foo".
The worst part of its use is that buf and ret could be different,
which means you cannot safely do:
char *buf = xmalloc(...);
buf = git_snpath(buf, n, "%s", ...);
... use buf ...
free(buf);
but instead have to do something like:
char *path, *buf = xmalloc(...);
path = git_snpath(buf, n, "%s", ...);
... use path ...
free(buf);
And this series goes in a direction of making more callers aware of
the twisted calling convention, which smells really bad.
As long as the callers are contained within this file, it is not
making things _too_ bad, so...
--
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