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.
Is it the same issue ? Or is it different because it's not a pointer cast?
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); ?
--
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