Messages in this thread | | | Subject | Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print_memmap | From | Andy Shevchenko <> | Date | Mon, 25 Jan 2016 10:31:53 +0200 |
| |
On Sat, 2016-01-23 at 10:03 -0800, James Bottomley wrote: > On Sat, 2016-01-23 at 19:18 +0200, Andy Shevchenko wrote: > > On Sat, Jan 23, 2016 at 6:44 PM, James Bottomley > > <James.Bottomley@hansenpartnership.com> wrote: > > > On Sat, 2016-01-23 at 16:55 +0200, Andy Shevchenko wrote: > > > > > > +static char * __init efi_size_format(char *buf, size_t size, > > > > u64 > > > > bytes) > > > > +{ > > > > + unsigned long i = bytes ? __ffs64(bytes) / 10 : 0; > > > > > > What if size is zero, which might happen on a UEFI screw up? > > > > size of what? Of input buffer? > > I mean when bytes == 0 ffs is undefined.
Well, someone misread the above code ;-)
There is ternary operator exactly to serve this purpose.
> > > > Also it gives really odd results for non power of two memory > > > sizes. 16384MB prints as 16GiB but 16385 prints as 16385MiB. > > > > Adaptive precision. I don't think the idea is to print a nearby > > numbers here. > > Well either there's a point to reducing to the nearest exponent or we > simply print everything in MB as the original did. Doing it > inconsistently is asking for trouble ... and lots of user queries. I > mean, supposing there's a range off by one ... now we print a huge > number in B.
> I really advise against hacking around like this. In any event if > efi > must have this, please don't involve the parts of the kernel that try > to do this correctly, like lib/string_helpers.h
> > > > If the goal is to have a clean interface reporting only the > > > first > > > four significant figures and a size exponent, then a helper > > > would > > > be much better than trying to open code this ad hoc. > > > > No. You get it wrong. The initial idea was (actually not mine, see > > authorship) to print an exact number with units and reduce whenever > > it's possible, i.e number is a multiplication of certain unit. > > so you must implement the original idea no matter how inconsistent it > leads us to be? Is it wrong to try to do better?
For both comments I prefer to hear Matt's opinion as he is maintainer of EFI stuff.
My role in this all is to reduce the code base by avoiding 'not invented here' syndrome.
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |