Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

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

 



On Wed, Sep 16, 2015 at 12:19:10PM -0700, Stefan Beller wrote:

> > 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.
> 
> So we could just not use the bracket notation, but the pointer then?
> 
>     xsnprintf(ownbuf[stage], sizeof(*ownbuf), "%o", ...);

IMHO that is even more confusing, as I expect sizeof(*ownbuf) to
generally be dereferencing a pointer to a struct, and we would be
writing to "ownbuf". There's not anything _wrong_ with what you've
written, it's just using a syntax that in my mind generally applies to a
different idiom, and I'd wonder if the writer got it wrong.

> IMHO that would reasonably well tell you that we just care about the
> size of one element there.
> 
> A funny thought:
> 
>      xsnprintf(ownbuf[stage], sizeof(ownbuf[-1]), "%o", ...);
> 
> should work as well as any reader would question the sanity
> of a negative index.

I'm not sure what the standard would have to say on that, as the
expression invokes undefined behavior (but of course we're not really
using the expression, only asking for the size). I tried to find any
mention of non-constant indices with sizeof() in the standard, but
couldn't.

I think at this point I'm inclined to switch it back to the original
sizeof(ownbuf[stage]), and we can deal with this if and when any
compiler actually complains.

-Peff
--
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]