Re: [PATCH v3 1/2] pkt-line: add stdio packet write functions
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 31, 2021, 18:13 UTC
- Message-ID
- <xmqqy28htrc6.fsf@gitster.g>
- In-Reply-To
- <20210831093444.28199-2-jacob@gitlab.com>
Jacob Vosmaer <jacob@gitlab.com> writes:
Show 5 quoted lines
> +/* > + * Stdio versions of packet_write functions. When mixing these with fd > + * based functions, take care to call fflush or packet_fflush before > + * doing fd writes or closing the fd. > + */
You may have wanted to say that fflush() is not needed immediately after packet_fflush() (because the latter calls fflush()), but I find the "or packet_fflush" in the above comment highly misleading.
It's not like you can randomly call packet_fflush() where you would call fflush(), as the former will insert a FLUSH packet to the output stream. "... take care to call fflush(3) before doihng fd writes or closing the fd" would be more appropriate. After all, if you make fflush() even after calling packet_fflush(), nothing will break.
Show 21 quoted lines
> +void packet_fwrite(FILE *f, const char *buf, size_t size);
> +void packet_fwrite_fmt(FILE *f, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
> +
> +/* packet_fflush writes a flush packet and flushes the stdio buffer of f */
> +void packet_fflush(FILE *f);
> +
> /*
> * Read a packetized line into the buffer, which must be at least size bytes
> * long. The return value specifies the number of bytes read into the buffer.
> diff --git a/write-or-die.c b/write-or-die.c
> index d33e68f6ab..7a2f84e2ee 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -70,3 +70,15 @@ void write_or_die(int fd, const void *buf, size_t count)
> die_errno("write error");
> }
> }
> +
> +void fwrite_or_die(FILE *f, const void *buf, size_t count)
> +{
> + if (fwrite(buf, count, 1, f) != 1)This counts the size of the buffer the wrong way.
fwrite() gives the size of each element to be written out first, and then number of elements that are written, which is the same as fread() but unfortunately the other way around from calloc() where count comes first X-<.
Other than that, nicely done.
Thanks.
Show 8 quoted lines
> + die_errno("fwrite error");
> +}
> +
> +void fflush_or_die(FILE *f)
> +{
> + if (fflush(f))
> + die_errno("fflush error");
> +}