Re: [PATCH v3 1/2] pkt-line: add stdio packet write functions

[Junio C Hamano <gitster@pobox.com>]

Jacob Vosmaer <jacob@gitlab.com> writes:

> +/*
> + * 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.

> +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.

> +		die_errno("fwrite error");
> +}
> +
> +void fflush_or_die(FILE *f)
> +{
> +	if (fflush(f))
> +		die_errno("fflush error");
> +}