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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 15 Jun 2010 07:07:10 -0400, Jeff King <peff@xxxxxxxx> 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?

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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]