[PATCH] ubimkvol: add -m option to create volume of maximum size
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.
> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 836 bytes
Desc: This is a digitally signed message part.
More information about the busybox