On 15 Mar 2016 12:00, Bartosz Gołaszewski wrote: > 2016-03-14 21:09 GMT+01:00 Rich Felker <[email protected]>: > > On Mon, Mar 14, 2016 at 10:27:19AM -0400, Mike Frysinger wrote: > >> On 14 Mar 2016 11:07, Bartosz Golaszewski wrote: > >> > +#ifndef __BB_NAMESPACE_H > >> > +#define __BB_NAMESPACE_H > >> > >> use a naming style like other busybox headers > > > > And in particular, don't use leading underscores, ever. They're not > > available for use by applications. > > > > I'll fix it in v5. May I ask what the issue is with leading > underscores? A lot of applications use them to distinguish routines > that are visible outside of their .c files, but are not part of any > API and shouldn't be called directly. Is this approach wrong?
__ falls into nebulous "reserved" namespace which means a compiler or C library could use that and it wouldn't be their fault if there was a collision in application code. on the other hand, busybox is a program all by itself (for the most part) and uses the BB_ namespace everywhere. ignoring that, if you look at the headers already in the tree, we don't use __BB_xxx. so it's already wrong purely for consistency reasons. > 2016-03-14 22:24 GMT+01:00 Mike Frysinger <[email protected]>: > > On 14 Mar 2016 16:15, Rich Felker wrote: > >> On Mon, Mar 14, 2016 at 02:27:56PM -0400, Mike Frysinger wrote: > >> > > >> > On 14 Mar 2016 18:11, Bartosz Gołaszewski wrote: > >> > the "actual" value is one higher than you might expect because pid_t > >> > is a signed quantity. for 8bit, "-128" is 0x80 and takes 4 bytes. > >> > for 32bit ("int"), "-2147483648" is 0x80000000 and takes 11 bytes. > >> > >> While pid_t is signed, negative values are not meaningful as pids; > >> they're pgids or special sentinels in some contexts. If this code is > >> trying to print a negative pid into a pathname used in /proc, that's a > >> bug. But in general, when using the 3*sizeof idiom you should add +1 > >> for a sign if needed and another +1 for null termination if not > >> already included elsewhere. > > > > relying on full error checking all the time is a bad way to design > > string buffers. "this func will smash the stack if you happen to > > pass in a negative value" is terrible. > > > > the NUL byte is technicallly already counted when you do the > > sizeof("/proc/..."). > > > > but this esoteric logic is why i'm pushing for something better than > > sprinkling ad-hoc logic everywhere and hoping for the best. > > Actually some time ago I sent a series extending the readahead applet. > It was rejected eventually but one of the patches was actually adding > a useful macro taken from systemd that seems to be doing a nice job at > determining the right size for string buffers holding integers: > > http://lists.busybox.net/pipermail/busybox/2015-August/083302.html > > And Denys' reponse: > > 2015-08-25 14:52 GMT+02:00 Denys Vlasenko <[email protected]>: > > > > I am using a mich simpler expression, sizeof(type)*3. > > > > type, sizeof(type)*3, actual reqd # of chars > > char 3 3 > > short 6 5 > > int 12 10 > > int64_t 24 19 > > > > As you see, it is a quite good approximation. > > > > Let's see how it works in real-world example: > > > > char path[sizeof("/proc/self/fd/%d") + sizeof(int)*3]; > > char path[sizeof("/proc/self/fd/%d") + DECIMAL_STR_MAX(int)]; > > > > With current name, it becomes longer. > > Can you make this macro shorter? i think his example just goes to show that, even when you think about it, you get it wrong :). his examples miss the extra byte needed for the - sign, and he used %d which is signed (rather than %u which is unsigned). also, that macro is wrong for the same reason :). -mike
signature.asc
Description: Digital signature
_______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
