Jeff King <peff@xxxxxxxx> writes:
> On Wed, Sep 16, 2015 at 11:24:27AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@xxxxxxxx> writes:
>>
>> > On Tue, Sep 15, 2015 at 11:19:21PM -0400, Eric Sunshine wrote:
>> >
>> >> > strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
>> >> > - sprintf(ownbuf[stage], "%o", ce->ce_mode);
>> >> > + xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);
>> >>
>> >> Interesting. I wonder if there are any (old/broken) compilers which
>> >> would barf on this. If we care, perhaps sizeof(ownbuf[0]) instead?
>> >
>> > Good point. I've changed it to sizeof(ownbuf[0]).
>>
>> Panda brain is lost here. What's the difference, other than that we
>> will now appear to be measuring the size of the thing at index 0
>> while using that size to stuff data into a different location? All
>> elements of the array are of the same size so there wouldn't be any
>> difference either way, no?
>
> Correct. The original is sane and gcc does the right thing. The question
> is whether some compiler would complain that "stage" is not a constant
> in the sizeof() expression. I don't know if any compiler would do so,
> but it is easy enough to be conservative.
Wouldn't such a compiler also complain if you did this, though?
int *pointer_to_int;
size_t sz = sizeof(*pointer_to_int);
You (as a complier) do not know exactly where ownbuf[stage] is,
because "stage" is unknown to you. In the same way, you do not know
exactly where *pointer_to_int is. But of course, what the sizeof()
operator is being asked is the size of the thing, which does not
depend on where the thing is. If you (as a compiler) does not know
that and complain to ownbuf[stage], wouldn't you complain to the
pointer dereference, too?
A more important reason I am reluctant to see this:
xsnprintf(ownbuf[stage], sizeof(ownbuf[0]), "%o", ...);
is that it looks strange in the same way as this
memcpy(ownbuf[stage], src, sizeof(ownbuf[0]));
looks strange. "We use the size of one thing to stuff into another".
That will make future readers wonder "Is this a typo, and if it is,
which index is a mistake I can fix?" and may lead to an unnecessary
confusion. I do not want to see a correctly written
xsnprintf(ownbuf[stage], sizeof(ownbuf[0]), "%o", ...);
turned into
xsnprintf(ownbuf[0], sizeof(ownbuf[0]), "%o", ...);
out of such a confusion.
--
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