Re: [PATCH v1 0/2] kstrtox: make _parse_integer() flexible

From: Andy Shevchenko

Date: Wed Jun 03 2026 - 10:02:56 EST


On Wed, Jun 03, 2026 at 12:51:09PM +0100, Rodrigo Alencar wrote:
> On 26/06/03 01:23PM, Petr Mladek wrote:
> > On Tue 2026-06-02 22:29:45, Andy Shevchenko wrote:
> > > Currently every new wrapper on _parse_integer_limit() will need a new name
> > > to share with users while keeping some optional arguments to be initialised
> > > explicitly. Since there is an attempt to expand this more, I decided to
> > > suggest this mini series to avoid namespace pollution and unneeded churn in
> > > the future.
> > >
> > > To expand this API more, the possible future change may be:
> > >
> > > unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned long long *res,
> > > - size_t max_chars);
> > > + size_t max_chars, $new_opt_arg);
> > >
> > > #define _parse_integer0(s, base, res, ...) \
> > > - _parse_integer_limit(s, base, res, INT_MAX);
> > > + _parse_integer_limit(s, base, res, INT_MAX, $new_opt_arg=$default);
> > >
> > > #define _parse_integer1(s, base, res, max_chars, ...) \
> > > - _parse_integer_limit(s, base, res, max_chars);
> > > + _parse_integer_limit(s, base, res, max_chars, $new_opt_arg=$default);
> > >
> > > +#define _parse_integer2(s, base, res, max_chars, new_opt_arg, ...) \
> > > + _parse_integer_limit(s, base, res, max_chars, new_opt_arg);
> >
> > I guess that this is about _parse_integer_limit_init() from
> > https://lore.kernel.org/all/20260531-adf41513-iio-driver-v15-2-da09adf1c0dd@xxxxxxxxxx/
> >
> > I personally find
> >
> > _parse_integer(*s, base. *res)
> > _parse_integer_limit(*s, base, *res, max_chars)
> > _parse_integer_limit_init(*s, base, init, *res, max_chars)
>
> What could also be done is having a _parse_integer_ext() that would
> contain all the necessary arguments (one that should be indicated
> not to be used) and define all the other variations as static inline
> functions. I suppose that combines both ideas, being a pattern that
> is also used elsewhere:
>
> devm_regulator_get()
> devm_regulator_get_enable()
> devm_regulator_get_enable_optional()

devm_regulator_get*() are public APIs and that split might make sense. The
_parse_integer() is internal to printf() implementation and wrappers around it,
so I don't think we should really be so picky. TL;DR: I think my approach is
good enough and makes not much of turbulence here, perhaps a comment would be
nice to have to explain the list of optional arguments.

> > a bit more self-explanatory than
> >
> > _parse_integer(*s, base. *res)
> > _parse_integer(*s, base, *res, max_chars)
> > _parse_integer(*s, base, *res, max_chars, init)
> >
> > especially when the meaning of the arguments need not be obvious
> > when the function is used in the code.
> >
> > Also the macro magic increases the complexity of the code.
> >
> > On the other hand, I do not have strong opinion. It is not a big deal.
> > I am not going to block it.

Would you give your tag in case Rodrigo wants to incorporate these into his
series and go via IIO tree? (I think that these two makes sense to put into
immutable branch/tag and share with the users).

> > > So you got the idea. (It is roughly overloaded function in OOP.)

--
With Best Regards,
Andy Shevchenko