linux-kernel - Re: [PATCH v3 3/4] x86/efi: print size in binary units in efi_print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1453572191.2470.52.camel@HansenPartnership.com>
Date:	Sat, 23 Jan 2016 10:03:11 -0800
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Matt Fleming <matt@...eblueprint.co.uk>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H . Peter Anvin" <hpa@...or.com>, linux-efi@...r.kernel.org,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel @ vger . kernel . org" <linux-kernel@...r.kernel.org>,
	Robert Elliott <elliott@....com>
Subject: Re: [PATCH v3 3/4] x86/efi: print size in binary units in
 efi_print_memmap

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@...senpartnership.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.

> >  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?

James