Jeff King wrote:
> On Fri, May 22, 2009 at 03:56:20AM -0400, Jeff King wrote:
>
>> Yeah, it is fine because it just passes the result to prep_temp_blob,
>> which respects the length. I don't know if it is worth making it more
>> safe (arguably it should just be using strbuf_readlink anyway, but that
>> does introduce an extra malloc).
>
> And here is the strbuf_readlink version, which actually does make the
> source shorter and easier to read.
>
> -- >8 --
> Subject: [PATCH] convert bare readlink to strbuf_readlink
>
> This particular readlink call never NUL-terminated its
> result, making it a potential source of bugs (though there
> is no bug now, as it currently always respects the length
> field). Let's just switch it to strbuf_readlink which is
> shorter and less error-prone.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> diff.c | 10 +++-------
> 1 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index f06876b..ffbe5c4 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2014,14 +2014,10 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
> die("stat(%s): %s", name, strerror(errno));
> }
> if (S_ISLNK(st.st_mode)) {
> - int ret;
> - char buf[PATH_MAX + 1]; /* ought to be SYMLINK_MAX */
> - ret = readlink(name, buf, sizeof(buf));
> - if (ret < 0)
> + struct strbuf sb = STRBUF_INIT;
> + if (strbuf_readlink(&sb, name, st.st_size) < 0)
> die("readlink(%s)", name);
> - if (ret == sizeof(buf))
> - die("symlink too long: %s", name);
> - prep_temp_blob(name, temp, buf, ret,
> + prep_temp_blob(name, temp, sb.buf, sb.len,
> (one->sha1_valid ?
> one->sha1 : null_sha1),
> (one->sha1_valid ?
Don't you need to strbuf_release() ?
-brandon
--
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