> On 10 Aug 2016, at 15:37, Jeff King <p...@peff.net> wrote:
> 
> On Wed, Aug 10, 2016 at 03:29:26PM +0200, Lars Schneider wrote:
> 
>> 
>>> On 10 Aug 2016, at 15:15, Jeff King <p...@peff.net> wrote:
>>> 
>>> On Wed, Aug 10, 2016 at 03:03:59PM +0200, larsxschnei...@gmail.com wrote:
>>> 
>>>> From: Lars Schneider <larsxschnei...@gmail.com>
>>>> 
>>>> format_packet() dies if the caller wants to format a packet larger than
>>>> LARGE_PACKET_MAX. Certain callers might prefer an error response instead.
>>> 
>>> I am not sure I agree here. Certainly I see the usefulness of gently
>>> handling a failure to write(). But if you are passing in too-large
>>> buffers, isn't that a bug in the program?
>>> 
>>> How would you recover, except by splitting up the content? That might
>>> not be possible depending on how you are using the pkt-lines. And even
>>> if it is, wouldn't it be simpler to split it up before sending it to
>>> format_packet()?
>> 
>> Good argument. I agree - this patch should be dropped.
> 
> Actually, after reading further, one thought did occur to me. Let's say
> you are writing to a smudge filter, and one of the header packets you
> send has the filename in it. So you might do something like:
> 
>  if (packet_write_fmt_gently(fd, "filename=%s", filename) < 0) {
>       if (filter_required)
>               die(...);
>       else
>               return -1; /* we tried our best; skip smudge */
>  }
> 
> The "recovery" there is not to try sending again, but rather to give up.
> And that is presumably a sane outcome for somebody who tries to checkout
> a filename larger than 64K.

Yes!


> It does still feel a little weird that you cannot tell the difference
> between a write() error and bad input. Because you really might want to
> do something different between the two. Like:
> 
>  #define MAX_FILENAME (PKTLINE_DATA_MAXLEN - strlen("filename"))
> 
>  if (filename > MAX_FILENAME) {
>       warning("woah, that name is ridiculous; truncating");
>       ret = packet_write_fmt_gently(fd, "%.*s", MAX_FILENAME, filename);
>  } else
>       ret = packet_write_fmt_gently(fd, "%s", filename);


I can do that. However, I wouldn't truncate the filename as this
might create a weird outcome. I would just let the filter fail.

OK?

- Lars--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to