Re: [PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()
> On 11 Sep 2016, at 18:01, Stefan Beller wrote: > > On Sun, Sep 11, 2016 at 4:36 AM, Lars Schneider > wrote: > >>> >>> call check_pipe from write_or_die here instead of >>> reproducing that function? >> [...] > >> Maybe it would be more suitable to move check_pipe to >> run-command.h/c? > > That's certainly possible. > I don't have a strong opinion, where the code actually > resides, but I do have a strong-ish opinion on code > duplication. ;) OK, then I will move check_pipe() to run-command. Thanks, Lars
Re: [PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()
On Sun, Sep 11, 2016 at 4:36 AM, Lars Schneider wrote: >> >>call check_pipe from write_or_die here instead of >>reproducing that function? > > Yes, might be better. I wasn't sure because the check_pipe is > not public. > > Where would you declare check_pipe? In cache.h? IIRC, once upon a time the community decided to not clutter cache.h any more as it is like a dirty kitchen sink, piling up all unrelated things, but on the other hand that would be handy. > Maybe it would be more suitable to move check_pipe to > run-command.h/c? That's certainly possible. I don't have a strong opinion, where the code actually resides, but I do have a strong-ish opinion on code duplication. ;)
Re: [PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()
> On 08 Sep 2016, at 23:18, Stefan Beller wrote:
>
> On Thu, Sep 8, 2016 at 11:21 AM, wrote:
>
>> +static int packet_write_fmt_1(int fd, int gently,
>> + const char *fmt, va_list args)
>> +{
>> + struct strbuf buf = STRBUF_INIT;
>> + size_t count;
>> +
>> + format_packet(&buf, fmt, args);
>> + count = write_in_full(fd, buf.buf, buf.len);
>> + if (count == buf.len)
>> + return 0;
>> +
>> + if (!gently) {
>
>call check_pipe from write_or_die here instead of
>reproducing that function?
Yes, might be better. I wasn't sure because the check_pipe is
not public.
Where would you declare check_pipe? In cache.h?
Maybe it would be more suitable to move check_pipe to
run-command.h/c?
>> + if (errno == EPIPE) {
>> + if (in_async())
>> + async_exit(141);
>> +
>> + signal(SIGPIPE, SIG_DFL);
>> + raise(SIGPIPE);
>> + /* Should never happen, but just in case... */
>> + exit(141);
>> + }
>> + die_errno("packet write error");
>> + }
>> + error("packet write failed");
>> + return -1;
>
> I think the more idiomatic way is to
>
>return error(...);
>
> as error always return -1.
Of course!!
Thank you,
Lars
Re: [PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()
On Thu, Sep 8, 2016 at 11:21 AM, wrote:
> +static int packet_write_fmt_1(int fd, int gently,
> + const char *fmt, va_list args)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + size_t count;
> +
> + format_packet(&buf, fmt, args);
> + count = write_in_full(fd, buf.buf, buf.len);
> + if (count == buf.len)
> + return 0;
> +
> + if (!gently) {
call check_pipe from write_or_die here instead of
reproducing that function?
> + if (errno == EPIPE) {
> + if (in_async())
> + async_exit(141);
> +
> + signal(SIGPIPE, SIG_DFL);
> + raise(SIGPIPE);
> + /* Should never happen, but just in case... */
> + exit(141);
> + }
> + die_errno("packet write error");
> + }
> + error("packet write failed");
> + return -1;
I think the more idiomatic way is to
return error(...);
as error always return -1.
[PATCH v7 03/10] pkt-line: add packet_write_fmt_gently()
From: Lars Schneider
packet_write_fmt() would die in case of a write error even though for
some callers an error would be acceptable. Add packet_write_fmt_gently()
which writes a formatted pkt-line and returns `0` for success and `-1`
for an error.
Signed-off-by: Lars Schneider
---
pkt-line.c | 43 +++
pkt-line.h | 1 +
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index e8adc0f..3824d05 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -125,16 +125,51 @@ static void format_packet(struct strbuf *out, const char
*fmt, va_list args)
packet_trace(out->buf + orig_len + 4, n - 4, 1);
}
+static int packet_write_fmt_1(int fd, int gently,
+ const char *fmt, va_list args)
+{
+ struct strbuf buf = STRBUF_INIT;
+ size_t count;
+
+ format_packet(&buf, fmt, args);
+ count = write_in_full(fd, buf.buf, buf.len);
+ if (count == buf.len)
+ return 0;
+
+ if (!gently) {
+ if (errno == EPIPE) {
+ if (in_async())
+ async_exit(141);
+
+ signal(SIGPIPE, SIG_DFL);
+ raise(SIGPIPE);
+ /* Should never happen, but just in case... */
+ exit(141);
+ }
+ die_errno("packet write error");
+ }
+ error("packet write failed");
+ return -1;
+}
+
void packet_write_fmt(int fd, const char *fmt, ...)
{
- static struct strbuf buf = STRBUF_INIT;
va_list args;
- strbuf_reset(&buf);
va_start(args, fmt);
- format_packet(&buf, fmt, args);
+ packet_write_fmt_1(fd, 0, fmt, args);
+ va_end(args);
+}
+
+int packet_write_fmt_gently(int fd, const char *fmt, ...)
+{
+ int status;
+ va_list args;
+
+ va_start(args, fmt);
+ status = packet_write_fmt_1(fd, 1, fmt, args);
va_end(args);
- write_or_die(fd, buf.buf, buf.len);
+ return status;
}
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
diff --git a/pkt-line.h b/pkt-line.h
index 1902fb3..3caea77 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -23,6 +23,7 @@ void packet_flush(int fd);
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format
(printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+int packet_write_fmt_gently(int fd, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
/*
* Read a packetized line into the buffer, which must be at least size bytes
--
2.10.0
