On Tue, 15 Jun 2010 05:54:53 -0400, Jeff King <peff@xxxxxxxx> wrote:
> On Tue, Jun 15, 2010 at 11:29:57AM +0200, Clément Poulain wrote:
>
>> > The same issue exists in Clément's patch to builtin/cat-file.c.
>>
>> We did this way because we found a similar cast in prep_temp_blob(),
>> diff.c:
>>
>> if (convert_to_working_tree(path,
>> (const char *)blob, (size_t)size, &buf)) {
>>
>> where size is an unsigned long.
>> Is it the same issue ? Or is it different because it's not a pointer
>> cast?
>
> Right. The compiler will handle conversion between integer types during
> assignment itself, converting representations as necessary (in fact,
> that cast looks useless to me, as implicit conversions are allowed in
> C). The only problem is dereferencing a pointer to X as something other
> than X.
>
>> Otherwise, we thought of reversing the conversion. That is to say,
>> instead
>> of casting "long *" in "size_t *" when calling textconv_object(), is it
>> better to cast size_t in "unsigned long" in textconv_object():
>>
>> *buf_size = (unsigned long) fill_textconv(textconv, df, buf); ?
>
> You shouldn't even have to cast there, for the same reason as above.
> That is why I wrote fill_textconv to return the size parameter, rather
> than writing to a passed-in pointer. It avoids the annoying
> size_t / unsigned long casting caused by different usage (in an ideal
> world, all of our sizes would be the same type, but the strbuf and diff
> code obviously differ).
Thanks for your answer.
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.
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?
Our gcc doesn't report any strict-aliasing problem, so we don't know if it
is better than the initial version or not...
--
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