On Friday 31 May 2013 22:08:17 Paul B. Henson wrote: > On 5/31/2013 4:31 PM, Mike Frysinger wrote: > > seems like all of these sscanf would be better off using the existing > > xstrtou() helpers ? > > When I'm making minor changes to an existing code base, I usually try to > match as much as possible the existing code. The current ubi_tools.c > uses sscanf like this on lines 239 and 245. I'm certainly open to > updating my patch to use xstrtou if that's preferable, it won't match > the existing code but I suppose another commit at some point could > change that.
yes, sometimes bad code gets merged and we should fix it rather than continue the practice :) > For reading leb_avail and leb_size it should be mostly a > drop-in replacement, but for picking the embedded number out from a > character string with leading text, what would you like to see? > Something like: > > if (strncmp(ubi_ctrl, "/dev/ubi", 8) == 0) > ubinum = xstrtou(ubi_ctrl+8, 10) whatever is easiest > The other uses I see of xstrtou don't really seem to be checking its > return value. It looks like if the number is too big to fit, it returns > UINT_MAX and sets errno = ERANGE, but what does it do if it doesn't find > some valid number or has some other problem? the x* funcs abort when they fail to process things. that's what the "x" at the start of the func name implies. see libbb/xatonum_template.c > > you sprintf the same string at the start of buf every time. this would > > probably be smaller code: > > While that's technically true, "every" is only "twice" :). yeah, the duplicate sscanf+die tricked my eyes > If you think > factoring out the constant string is worth it, I'll go ahead and update > the patch with the technique you recommended. the right answer is whatever is smaller. my suggestion ends up packing strings tighter. -mike
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
