Re: [PATCH v2 2/3] textconv: support for blame

[bonneta <bonneta@ensimag.fr>]

On Tue, 15 Jun 2010 07:07:10 -0400, Jeff King <peff@peff.net> wrote:
> [resending to cc git@vger]
> 
> On Tue, Jun 15, 2010 at 12:29:43PM +0200, bonneta wrote:
> 
>> We have changed the declaration of textconv_object() to:
>> 
>> static int textconv_object(const char *path,
>>                            const unsigned char *sha1,
>>                            char **buf,
>>                            unsigned long *buf_size)
>> 
>> And now we can do:
>> *buf_size = fill_textconv(textconv, df, buf);
>> without any cast.
> 
> I assume you mean dropping the final buf_size parameter from that
> declaration, which is what your usage example has. I would return either
> an "unsigned long" or a size_t rather than an int. We are dealing with
> potential whole-file sizes, so it is better to use at least as large a
> data type as other parts of the code (we still may run into truncation
> problems, but at least you are not making things any worse).
> 
>> But we have to do:
>> textconv_object(read_from, null_sha1, &buf.buf, (unsigned long *)
>> &buf.len))
>> where buf.len is size_t.
>> 
>> Is that ok?
> 
> No, that has the same problem. Imagine a big endian machine with a
> 32-bit unsigned long and a 64-bit size_t. You would write into the first
> 32 bits of buf.len, which are the high bits, giving you a ridiculously
> large answer.
> 
> The only portable way in C to convert between types is by assignment. So
> you have to do:
> 
>   unsigned long foo;
>   textconv_object(read_from, null_sha1, &buf.buf, &foo);
>   buf.len = foo;
> 
> But now I'm confused. That matches the declaration you gave in the first
> part of your email, but not the usage example.

We now understand what we have to do, thank you.
We are currently fixing this patch.
Do we have to resend only this patch or the whole series?