linux-kernel - Re: [PATCH v4] lib: add size unit t/p/e to memparse

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: <alpine.DEB.2.02.1406161828180.21018@chino.kir.corp.google.com>
Date:	Mon, 16 Jun 2014 18:29:48 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Gui Hecheng <guihc.fnst@...fujitsu.com>
cc:	linux-kernel@...r.kernel.org, linux-btrfs@...r.kernel.org,
	akpm@...ux-foundation.org
Subject: Re: [PATCH v4] lib: add size unit t/p/e to memparse

On Tue, 17 Jun 2014, Gui Hecheng wrote:

> > > diff --git a/lib/cmdline.c b/lib/cmdline.c
> > > index d4932f7..76a712e 100644
> > > --- a/lib/cmdline.c
> > > +++ b/lib/cmdline.c
> > > @@ -121,11 +121,7 @@ EXPORT_SYMBOL(get_options);
> > >   *	@retptr: (output) Optional pointer to next char after parse completes
> > >   *
> > >   *	Parses a string into a number.  The number stored at @ptr is
> > > - *	potentially suffixed with %K (for kilobytes, or 1024 bytes),
> > > - *	%M (for megabytes, or 1048576 bytes), or %G (for gigabytes, or
> > > - *	1073741824).  If the number is suffixed with K, M, or G, then
> > > - *	the return value is the number multiplied by one kilobyte, one
> > > - *	megabyte, or one gigabyte, respectively.
> > > + *	potentially suffixed with K, M, G, T, P, E.
> > >   */
> > >  
> > >  unsigned long long memparse(const char *ptr, char **retptr)
> > > @@ -135,6 +131,15 @@ unsigned long long memparse(const char *ptr, char **retptr)
> > >  	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
> > >  
> > >  	switch (*endptr) {
> > > +	case 'E':
> > > +	case 'e':
> > > +		ret <<= 10;
> > > +	case 'P':
> > > +	case 'p':
> > > +		ret <<= 10;
> > > +	case 'T':
> > > +	case 't':
> > > +		ret <<= 10;
> > >  	case 'G':
> > >  	case 'g':
> > >  		ret <<= 10;
> > 
> > Seems fine since unsigned long long is always at least 64 bits, but 
> > perhaps also change simple_strtoull() to use kstrtoull() at the same time 
> > since the former is deprecated?
> 
> Yes, that is a point. But the deprecated function is a separate problem
> and may not be included in this patch.
> Also, I find that simple_strtoull is used in many places in the kernel
> code, it is better to replace it globally?
> 

If you're going to have a go at replacing the simple_strto*() functions 
throughout the kernel, it's probably better to do it per subsystem (as 
defined by the separate sections of MAINTAINERS) and propose the patches 
individually to those maintainers.  Once it has been removed entirely, you 
can submit a patch to remove the functions themselves.

Be aware that there are many callers to the deprecated functions so it may 
take a significant amount of time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/