[PATCH] ubimkvol: add -m option to create volume of maximum size

Mike Frysinger vapier at gentoo.org
Sat Jun 1 03:05:56 UTC 2013


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20130531/2f303578/attachment.asc>


More information about the busybox mailing list