Re: [PATCH v7 2/2] http-backend: respect CONTENT_LENGTH for receive-pack

[Max Kirillov <max@max630.net>]

On Tue, Jun 05, 2018 at 01:18:08AM +0300, Max Kirillov wrote:
> On Mon, Jun 04, 2018 at 12:44:09AM -0400, Jeff King wrote:
>> Since this is slightly less efficient, and because it only matters if
>> the web server does not already close the pipe, should this have a
>> run-time configuration knob, even if it defaults to
>> safe-but-slightly-slower?
> 
> Personally, I of course don't want this. Also, I don't think
> the difference is much noticeable. But you can never be sure
> without trying. I'll try to measure some numbers.

It seems to be challenging to see any effect at my system.
At least not with any real operation because changing
references needs IO and index-pack needs CPU so. I'll try
it some more.

>> We should probably say something more generic like:
>> 
>>   die_errno("unable to write to '%s'");
>> 
>> or similar.
> 
> Actually, it is already 3rd same error in this file. Maybe
> deserve some refactoring. I will change the message also.

Extracted the writing and refactoring to a single function,
also fixed the message.

>>> +cat >fetch_body <<EOF
>>> +0032want $hash_head
>>> +00000032have $hash_prev
>>> +0009done
>>> +EOF
>> 
>> This depends on the size of the hash. That's always 40 for now, but is
>> something that may change soon.
>> 
>> We already have a packetize() helper; could we use it here?

> Could you point me to it? I cannot find it.

Sorry, misread it as packetSize. Found and used.

>> Also, do we need to protect ourselves against other signals being
>> delivered? E.g., if I resize my xterm and this process gets SIGWINCH, is
>> it going to erroneously end the sleep and say "nope, no exited signal"?

> I'll check, but what could I do? Should I add blocking other
> signals there?

In my Linux I don't see the signal. Except that, there seem to
be not that many ignored signals. Anyway, I don't see what
could be done bout it.