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
| ||
|
Message-ID: <20231227163832.51e305f7@echidna> Date: Wed, 27 Dec 2023 16:38:32 +1100 From: David Disseldorp <ddiss@...ba.org> To: Qu Wenruo <quwenruo.btrfs@....com> Cc: Qu Wenruo <wqu@...e.com>, linux-btrfs@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>, Christophe JAILLET <christophe.jaillet@...adoo.fr>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, David Laight <David.Laight@...LAB.COM>, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 1/2] lib/strtox: introduce kstrtoull_suffix() helper On Sat, 23 Dec 2023 20:27:30 +1030, Qu Wenruo wrote: > On 2023/12/20 16:08, David Disseldorp wrote: ... > >> +#define KSTRTOULL_SUFFIX_DEFAULT (SUFFIX_K | SUFFIX_M | SUFFIX_G | SUFFIX_T | SUFFIX_P) > > > > I think it'd be clearer if you dropped this default and had callers > > explicitly provide the desired suffix mask. > > Well, that would be long, and would be even longer as the newer naming > would be MEMPARSE_SUFFIX_*, to be more explicit on what the suffix is for... > > And I really want callers to choose a saner default suffix, thus here > comes the default one. > > In fact, in my next version, I also found that there are some memparse() > call sites benefits from the newer suffixes (although won't for the "E" > one). > The example is the call site setup_elfcorehdr(). Where the comment only > mentions KMG, but since memparse() silently added "PE" suffixes, maybe > on some mainframes we saved some time for one or two lucky admins. I think it's a sane default, my concern is that _DEFAULT says nothing about supported units from the caller's perspective. Perhaps MEMPARSE_SUFFIX_KMGTP or MEMPARSE_UNITS_KMGTP would be clearer. ... > > With the above changes made, feel free to add > > Reviewed-by: David Disseldorp <ddiss@...e.de> > > Thanks for the review, but I'm afraid the newer version would be another > beast. > > All the ommitted comments would be addressed a in new series. > > > > I'll leave the review of patch 2/2 up to others, as I'm still a little > > worried about sysfs trailing whitespace regressions. > > That won't be a problem anymore, the new series would keep the old > @retptr behavior, thus for btrfs part it won't be changed at all. Sounds good. Will follow up there. Cheers, David
Powered by blists - more mailing lists