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]
Date: Tue, 13 Feb 2024 17:06:24 -0500
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Suren Baghdasaryan <surenb@...gle.com>, akpm@...ux-foundation.org, 
	mhocko@...e.com, vbabka@...e.cz, hannes@...xchg.org, roman.gushchin@...ux.dev, 
	mgorman@...e.de, dave@...olabs.net, willy@...radead.org, liam.howlett@...cle.com, 
	corbet@....net, void@...ifault.com, peterz@...radead.org, juri.lelli@...hat.com, 
	catalin.marinas@....com, will@...nel.org, arnd@...db.de, tglx@...utronix.de, 
	mingo@...hat.com, dave.hansen@...ux.intel.com, x86@...nel.org, peterx@...hat.com, 
	david@...hat.com, axboe@...nel.dk, mcgrof@...nel.org, masahiroy@...nel.org, 
	nathan@...nel.org, dennis@...nel.org, tj@...nel.org, muchun.song@...ux.dev, 
	rppt@...nel.org, paulmck@...nel.org, pasha.tatashin@...een.com, 
	yosryahmed@...gle.com, yuzhao@...gle.com, dhowells@...hat.com, hughd@...gle.com, 
	andreyknvl@...il.com, keescook@...omium.org, ndesaulniers@...gle.com, 
	vvvvvv@...gle.com, gregkh@...uxfoundation.org, ebiggers@...gle.com, 
	ytcoode@...il.com, vincent.guittot@...aro.org, dietmar.eggemann@....com, 
	rostedt@...dmis.org, bsegall@...gle.com, bristot@...hat.com, vschneid@...hat.com, 
	cl@...ux.com, penberg@...nel.org, iamjoonsoo.kim@....com, 42.hyeyoo@...il.com, 
	glider@...gle.com, elver@...gle.com, dvyukov@...gle.com, shakeelb@...gle.com, 
	songmuchun@...edance.com, jbaron@...mai.com, rientjes@...gle.com, minchan@...gle.com, 
	kaleshsingh@...gle.com, kernel-team@...roid.com, linux-doc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, iommu@...ts.linux.dev, linux-arch@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org, linux-modules@...r.kernel.org, 
	kasan-dev@...glegroups.com, cgroups@...r.kernel.org, Andy Shevchenko <andy@...nel.org>, 
	Michael Ellerman <mpe@...erman.id.au>, Benjamin Herrenschmidt <benh@...nel.crashing.org>, 
	Paul Mackerras <paulus@...ba.org>, "Michael S. Tsirkin" <mst@...hat.com>, 
	Jason Wang <jasowang@...hat.com>, Noralf Trønnes <noralf@...nnes.org>
Subject: Re: [PATCH v3 01/35] lib/string_helpers: Add flags param to
 string_get_size()

On Tue, Feb 13, 2024 at 10:26:48AM +0200, Andy Shevchenko wrote:
> On Mon, Feb 12, 2024 at 11:39 PM Suren Baghdasaryan <surenb@...gle.com> wrote:
> >
> > From: Kent Overstreet <kent.overstreet@...ux.dev>
> >
> > The new flags parameter allows controlling
> >  - Whether or not the units suffix is separated by a space, for
> >    compatibility with sort -h
> >  - Whether or not to append a B suffix - we're not always printing
> >    bytes.
> >
> > Signed-off-by: Kent Overstreet <kent.overstreet@...ux.dev>
> > Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> 
> ...
> 
> You can move the below under --- cutter, so it won't pollute the git history.
> 
> > Cc: Andy Shevchenko <andy@...nel.org>
> > Cc: Michael Ellerman <mpe@...erman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
> > Cc: Paul Mackerras <paulus@...ba.org>
> > Cc: "Michael S. Tsirkin" <mst@...hat.com>
> > Cc: Jason Wang <jasowang@...hat.com>
> > Cc: "Noralf Trønnes" <noralf@...nnes.org>
> > Cc: Jens Axboe <axboe@...nel.dk>
> > ---
> 
> ...
> 
> > --- a/include/linux/string_helpers.h
> > +++ b/include/linux/string_helpers.h
> > @@ -17,14 +17,13 @@ static inline bool string_is_terminated(const char *s, int len)
> 
> ...
> 
> > -/* Descriptions of the types of units to
> > - * print in */
> > -enum string_size_units {
> > -       STRING_UNITS_10,        /* use powers of 10^3 (standard SI) */
> > -       STRING_UNITS_2,         /* use binary powers of 2^10 */
> > +enum string_size_flags {
> > +       STRING_SIZE_BASE2       = (1 << 0),
> > +       STRING_SIZE_NOSPACE     = (1 << 1),
> > +       STRING_SIZE_NOBYTES     = (1 << 2),
> >  };
> 
> Do not kill documentation, I already said that. Or i.o.w. document this.
> Also the _SIZE is ambigous (if you don't want UNITS, use SIZE_FORMAT.
> 
> Also why did you kill BASE10 here? (see below as well)

As you should be able to tell from the name, it's a set of flags.

> > --- a/lib/string_helpers.c
> > +++ b/lib/string_helpers.c
> > @@ -19,11 +19,17 @@
> >  #include <linux/string.h>
> >  #include <linux/string_helpers.h>
> >
> > +enum string_size_units {
> > +       STRING_UNITS_10,        /* use powers of 10^3 (standard SI) */
> > +       STRING_UNITS_2,         /* use binary powers of 2^10 */
> > +};
> 
> Why do we need this duplication?

Because otherwise a lot more code would have to change.
> 
> It seems most of my points from the previous review were refused...

Look, Andy, this is a pretty tiny part of the patchset, yet it's been
eating up a pretty disproprortionate amount of time and your review
feedback has been pretty unhelpful - asking for things to be broken up
in ways that would not be bisectable, or (as here) re-asking the same
things that I've already answered and that should've been obvious.

The code works. If you wish to complain about anything being broken, or
if you can come up with anything more actionable than what you've got
here, I will absolutely respond to that, but otherwise I'm just going to
leave things where they sit.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ