Mike Hommey <mh@xxxxxxxxxxxx> writes:
> Also, replace whitespaces with tabs in some places
>
> Signed-off-by: Mike Hommey <mh@xxxxxxxxxxxx>
> ---
>
> While testing this, I noticed 3 things:
> - CURL_MULTI makes the code very racy
> - a lot of the code doesn't do anything useful without CURL_MULTI
> - the code is redundant
Yeah, there does seem to be a lot of duplicated code that does common
setup with slightly different request string.
> @@ -1115,16 +1109,11 @@ static char *quote_ref_url(const char *base, const char *ref)
>
> int fetch_ref(char *ref, unsigned char *sha1)
> {
> - char *url;
> - char hex[42];
> - struct buffer buffer;
> + char *url;
> + struct strbuf buffer = STRBUF_INIT;
> char *base = remote->url;
> struct active_request_slot *slot;
> struct slot_results results;
> - buffer.size = 41;
> - buffer.posn = 0;
> - buffer.buffer = hex;
> - hex[41] = '\0';
>
> url = quote_ref_url(base, ref);
> slot = get_active_slot();
> @@ -1142,9 +1131,9 @@ int fetch_ref(char *ref, unsigned char *sha1)
> return error("Unable to start request");
> }
>
> - hex[40] = '\0';
> - get_sha1_hex(hex, sha1);
> - return 0;
> + buffer.buf[40] = '\0';
> + get_sha1_hex(buffer.buf, sha1);
> + return 0;
> }
A conversion like this is worrysome and needs to be rethought I think.
At least with the old code, we knew hex[40] was a safe location to make
assignment to, even though we did not check if what it contained made
sense --- the other end might have had a garbage in that URL (but the
caller hopefully would be responsible for noticing that). But with your
change, I do not think you have that guarantee. fwrite_buffer() may
have extended the buffer using strbuf API, but it may have received less
than what you are expecting, in which case you may not have buf[40]
touchable for you, no?
I at the same time think the original code is buggy. It initializes
buffer.buffer to the on-stack storage hex[], but lets fwrite_buffer() to
call xrealloc() on it.
-
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
