Carlos Rica <jasampler@xxxxxxxxx> writes:
> The new name is closer to the purpose of the function.
>
> The other change just makes things easier for callers needing a
> NUL-terminated buffer.
>
> Since the function returns only the memory written with data,
> almost always allocating more space than needed because final
> size is unknown, an extra NUL terminating the buffer is harmless.
> It is not included in the returned size, so the function
> remains working as before.
>
> Also, now the function allows the buffer passed to be NULL at first,
> and alloc_nr is now used for growing the buffer, instead size=*2.
Some people may say function name change should be a separate
patch by itself, but I'd let it pass for now...
> diff --git a/sha1_file.c b/sha1_file.c
> index 1efd9ae..563ec07 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2304,27 +2304,38 @@ int has_sha1_file(const unsigned char *sha1)
> *
> * returns 0 if anything went fine and -1 otherwise
> *
> + * The buffer is always NUL-terminated, not including it in returned size.
> + *
> * NOTE: both buf and size may change, but even when -1 is returned
> * you still have to free() it yourself.
> */
> -int read_pipe(int fd, char** return_buf, unsigned long* return_size)
> +int read_fd(int fd, char** return_buf, unsigned long* return_size)
> {
> char* buf = *return_buf;
Our code seems to prefer pointer declaration to be spelled as
"type **var", not "type** var".
> unsigned long size = *return_size;
> ssize_t iret;
> unsigned long off = 0;
>
> + if (!buf || size <= 1) {
> + size = alloc_nr(size);
> + buf = xrealloc(buf, size);
> + }
> +
Hmph. The reason this is not "!size" is because you would want
more than one. As your plan is to use this mostly for the -F
option of "tag/commit", I suspect using a bit larger default,
such as 80 (just a line), or probably 1k (most log messages
would fit in such a buffer), would be more practical.
> do {
> iret = xread(fd, buf + off, size - off);
> if (iret > 0) {
> off += iret;
> if (off == size) {
> - size *= 2;
> + size = alloc_nr(size);
> buf = xrealloc(buf, size);
> }
> }
> } while (iret > 0);
>
> + if (off == size)
> + buf = xrealloc(buf, size + 1);
> + buf[off] = '\0';
> +
I wonder if doing xread(... (size-1) - off) in the loop would
ensure (off <= size-1) here. You also would need to update the
realloc condition inside loop if you do 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
