Re: [PATCH v3 03/10] pkt-line: add packet_flush_gentle()
- From
Lars Schneider <larsxschneider@gmail.com>
- Date
- Jul 31, 2016, 21:45 UTC
- Message-ID
- <8FC2D283-AF8D-4643-834E-3D1927C558C0@gmail.com>
- In-Reply-To
- <63231F5B-959F-4A9D-89B9-E4A42AF34AB1@web.de>
Show 21 quoted lines
> 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
Show 43 quoted lines
>
>
>> 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
>>