Re: [PATCH 0/3] limit the size of the packs we receive

2016-08-16 Thread Jeff King
On Tue, Aug 16, 2016 at 04:44:01PM +0200, Christian Couder wrote:

>> [sizes and signedness of off_t]
> I can add something along your explanations in the commit message if
> it is prefered.

I think it's probably OK without it.

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


Re: [PATCH 0/3] limit the size of the packs we receive

2016-08-16 Thread Christian Couder
On Tue, Aug 16, 2016 at 3:11 PM, Jeff King  wrote:
> On Tue, Aug 16, 2016 at 10:16:58AM +0200, Christian Couder wrote:
>
>>   - changed strtoul() to strtoumax() in the first 2 patches, as
>> suggested by Peff,
>>
>>   - changed git_config_ulong() to git_config_int64() and used PRIuMAX
>> and uintmax_t in the last patch, as suggested by Peff,
>
> Thinking a bit, off_t is actually signed. So maybe PRIdMAX (which we
> don't seem to have compat macros for) would make more sense. I dunno if
> anybody actually cares. This value shouldn't be signed anyway, and
> nobody should be approaching the limits of a 64-bit number anyway (there
> is no point in limiting the incoming pack to the exabyte range).
>
> So I'm inclined not to worry about it.

Yeah, you previously wrote:

> We seem to use strtoumax() elsewhere, so that's probably a good match
> (technically it can overflow an off_t, but in practice this value comes from
> the admin and they will set something sane).

and I thought that the same would apply to using PRIuMAX and uintmax_t
in this patch.

I can add something along your explanations in the commit message if
it is prefered.
--
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


Re: [PATCH 0/3] limit the size of the packs we receive

2016-08-16 Thread Jeff King
On Tue, Aug 16, 2016 at 10:16:58AM +0200, Christian Couder wrote:

> Changes from previous RFC version
> ~
> 
>   - added documentation to all the 3 patches,

Good idea.

>   - changed strtoul() to strtoumax() in the first 2 patches, as
> suggested by Peff,
> 
>   - changed git_config_ulong() to git_config_int64() and used PRIuMAX
> and uintmax_t in the last patch, as suggested by Peff,

Thinking a bit, off_t is actually signed. So maybe PRIdMAX (which we
don't seem to have compat macros for) would make more sense. I dunno if
anybody actually cares. This value shouldn't be signed anyway, and
nobody should be approaching the limits of a 64-bit number anyway (there
is no point in limiting the incoming pack to the exabyte range).

So I'm inclined not to worry about it.

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