Clément Poulain <clement.poulain@xxxxxxxxxxxxxxx> writes:
> On Mon, 14 Jun 2010 13:40:21 -0700, Junio C Hamano <gitster@xxxxxxxxx>
> wrote:
>> Axel Bonnet <axel.bonnet@xxxxxxxxxxxxxxx> writes:
>>
>>> @@ -86,16 +87,49 @@ struct origin {
>>> ...
>>> +static void fill_origin_blob(struct diff_options *opt,
>>> + struct origin *o, mmfile_t *file)
>>> {
>>> if (!o->file.ptr) {
>>> enum object_type type;
>>> num_read_blob++;
>>> - file->ptr = read_sha1_file(o->blob_sha1, &type,
>>> - (unsigned long *)(&(file->size)));
>>> +
>>> + if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
>>> + textconv_object(o->path, o->blob_sha1, &file->ptr,
>>> + (size_t *) &file->size))
>>
>> This cast is not correct, as there is no guarantee that your size_t and
>> typeof(mmfile_t.size) are compatible. Depending on the gcc version, you
>> would get "dereferencing type-punned pointer will break strict-aliasing
>> rules" error.
>>
>> 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.
That is a completely different kind of cast that is sane. The function
takes a _value_ of type size_t.
Your cast is "This function wants to store a value to a _memory location_
that is supposed to hold a value of type size_t, and I know &(file->size)
is not such a location (it is to hold a value of type 'unsigned long');
please pretend that these two distinct pointers are compatible", which is
a big no-no.
--
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