Re: [PATCH v3 03/10] pkt-line: add packet_flush_gentle()

On 31 Jul 2016, at 22:36, Torstem Bögershausen [off-list ref] wrote:

quoted

Am 29.07.2016 um 20:37 schrieb larsxschneider@gmail.com:

From: Lars Schneider <redacted>

packet_flush() would die in case of a write error even though for some callers
an error would be acceptable.

What happens if there is a write error ?
Basically the protocol is out of synch.
Lenght information is mixed up with payload, or the other way
around.
It may be, that the consequences of a write error are acceptable,
because a filter is allowed to fail.
What is not acceptable is a "broken" protocol.
The consequence schould be to close the fd and tear down all
resources. connected to it.
In our case to terminate the external filter daemon in some way,
and to never use this instance again.

Correct! That is exactly what is happening in kill_protocol2_filter()
here:


+static int apply_protocol2_filter(const char *path, const char *src, size_t len,
+						int fd, struct strbuf *dst, const char *cmd,
+						const int wanted_capability)
+{
...
+	if (ret) {
+		strbuf_swap(dst, &nbuf);
+	} else {
+		if (!filter_result || strcmp(filter_result, "reject")) {
+			// Something went wrong with the protocol filter. Force shutdown!
+			error("external filter '%s' failed", cmd);
+			kill_protocol2_filter(&cmd_process_map, entry);
+		}
+	}
+	strbuf_release(&nbuf);
+	return ret;
+}

More context:
https://github.com/larsxschneider/git/blob/e128326070847ac596e8bb21adebc8abab2003fc/convert.c#L821

- Lars

quoted

Add packet_flush_gentle() which writes a pkt-line
flush packet and returns `0` for success and `1` for failure.

Signed-off-by: Lars Schneider <redacted>
---
pkt-line.c | 6 ++++++
pkt-line.h | 1 +
2 files changed, 7 insertions(+)

diff --git a/pkt-line.c b/pkt-line.c
index 6fae508..1728690 100644
--- a/pkt-line.c
+++ b/pkt-line.c

@@ -91,6 +91,12 @@ void packet_flush(int fd)

write_or_die(fd, "0000", 4);
}

+int packet_flush_gentle(int fd)
+{
+    packet_trace("0000", 4, 1);
+    return !write_or_whine_pipe(fd, "0000", 4, "flush packet");
+}
+
void packet_buf_flush(struct strbuf *buf)
{
packet_trace("0000", 4, 1);

diff --git a/pkt-line.h b/pkt-line.h
index 02dcced..3953c98 100644
--- a/pkt-line.h
+++ b/pkt-line.h

@@ -23,6 +23,7 @@ void packet_flush(int fd);

void packet_write(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_flush_gentle(int fd);
int direct_packet_write(int fd, char *buf, size_t size, int gentle);
int direct_packet_write_data(int fd, const char *buf, size_t size, int gentle);

-- 
2.9.0