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

From: Max Kirillov <max@max630.net>
Date: Jun 10, 2018, 15:06 UTC
Message-ID: <20180610150619.GD27650@jessie.local>
In-Reply-To: <20180604221807.GC27650@jessie.local>

On Tue, Jun 05, 2018 at 01:18:08AM +0300, Max Kirillov wrote:

Show 9 quoted lines

> 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.

Show 8 quoted lines

>> 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.

Show 10 quoted lines

>>> +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.