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

[Lars Schneider <larsxschneider@gmail.com>]

> On 31 Jul 2016, at 22:36, Torstem Bögershausen <tboegi@web.de> wrote:
> 
> 
> 
>> Am 29.07.2016 um 20:37 schrieb larsxschneider@gmail.com:
>> 
>> From: Lars Schneider <larsxschneider@gmail.com>
>> 
>> 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


> 
> 
>> 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 <larsxschneider@gmail.com>
>> ---
>> 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
>>