linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	lizefan@huawei.com, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	josh@joshtriplett.org, Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	fweisbec@gmail.com, Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: Re: [PATCH 5/8] lib: bitmap_getnum: separate arg into region and field
Date: Tue, 26 Jan 2021 18:58:46 -0800	[thread overview]
Message-ID: <CAAH8bW9pQ_==uF3_Zo6K-ijqHXDVcW_-9gx0fvFWXe=fEvsg9A@mail.gmail.com> (raw)
In-Reply-To: <YBCIXzUkgjcCTprv@smile.fi.intel.com>

On Tue, Jan 26, 2021 at 1:22 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Jan 26, 2021 at 12:11:38PM -0500, Paul Gortmaker wrote:
> > The bitmap_getnum is only used on a region's start/end/off/group_len
> > field.  Trivially decouple the region from the field so that the region
> > pointer is available for a pending change.
>
> Honestly, I don't like this macro trick. It's bad in couple of ways:
>  - it hides what actually is done with the fields of r structure
>    (after you get that they are fields!)
>  - it breaks possibility to compile time (type) checks
>
> I will listen what others say, but I'm in favour not to proceed like this.

Agree. Would be better to drop the patch. Paul, what kind of pending
change do you mean here? All the following patches are not related to
parsing machinery.

> > Cc: Yury Norov <yury.norov@gmail.com>
> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > ---
> >  lib/bitmap.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/bitmap.c b/lib/bitmap.c
> > index 833f152a2c43..f65be2f148fd 100644
> > --- a/lib/bitmap.c
> > +++ b/lib/bitmap.c
> > @@ -533,6 +533,7 @@ static const char *bitmap_getnum(const char *str, unsigned int *num)
> >       *num = n;
> >       return str + len;
> >  }
> > +#define bitmap_getrnum(s, r, pos) bitmap_getnum(s, &(r->pos))
> >
> >  static inline bool end_of_str(char c)
> >  {
> > @@ -571,7 +572,7 @@ static const char *bitmap_find_region_reverse(const char *start, const char *end
> >
> >  static const char *bitmap_parse_region(const char *str, struct region *r)
> >  {
> > -     str = bitmap_getnum(str, &r->start);
> > +     str = bitmap_getrnum(str, r, start);
> >       if (IS_ERR(str))
> >               return str;
> >
> > @@ -581,7 +582,7 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
> >       if (*str != '-')
> >               return ERR_PTR(-EINVAL);
> >
> > -     str = bitmap_getnum(str + 1, &r->end);
> > +     str = bitmap_getrnum(str + 1, r, end);
> >       if (IS_ERR(str))
> >               return str;
> >
> > @@ -591,14 +592,14 @@ static const char *bitmap_parse_region(const char *str, struct region *r)
> >       if (*str != ':')
> >               return ERR_PTR(-EINVAL);
> >
> > -     str = bitmap_getnum(str + 1, &r->off);
> > +     str = bitmap_getrnum(str + 1, r, off);
> >       if (IS_ERR(str))
> >               return str;
> >
> >       if (*str != '/')
> >               return ERR_PTR(-EINVAL);
> >
> > -     return bitmap_getnum(str + 1, &r->group_len);
> > +     return bitmap_getrnum(str + 1, r, group_len);
> >
> >  no_end:
> >       r->end = r->start;
> > --
> > 2.17.1
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

  reply	other threads:[~2021-01-27  4:02 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-26 17:11 [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation Paul Gortmaker
2021-01-26 17:11 ` [PATCH 1/8] lib: test_bitmap: clearly separate ERANGE from EINVAL tests Paul Gortmaker
2021-01-26 21:04   ` Andy Shevchenko
2021-01-27  7:21     ` Paul Gortmaker
2021-01-26 17:11 ` [PATCH 2/8] lib: test_bitmap: add more start-end:offset/len tests Paul Gortmaker
2021-01-26 21:11   ` Andy Shevchenko
2021-01-27  3:03   ` Yury Norov
2021-01-26 17:11 ` [PATCH 3/8] lib: bitmap: fold nbits into region struct Paul Gortmaker
2021-01-26 21:16   ` Andy Shevchenko
2021-01-26 21:18     ` Andy Shevchenko
2021-01-27  8:02     ` Paul Gortmaker
2021-01-28  0:47       ` Yury Norov
2021-01-28 10:17         ` Andy Shevchenko
2021-01-27  3:08   ` Yury Norov
2021-01-26 17:11 ` [PATCH 4/8] lib: bitmap: move ERANGE check from set_region to check_region Paul Gortmaker
2021-01-26 21:19   ` Andy Shevchenko
2021-01-27  3:12   ` Yury Norov
2021-01-26 17:11 ` [PATCH 5/8] lib: bitmap_getnum: separate arg into region and field Paul Gortmaker
2021-01-26 21:23   ` Andy Shevchenko
2021-01-27  2:58     ` Yury Norov [this message]
2021-01-27  8:38       ` Paul Gortmaker
2021-01-26 17:11 ` [PATCH 6/8] lib: bitmap: support "N" as an alias for size of bitmap Paul Gortmaker
2021-01-26 21:37   ` Andy Shevchenko
2021-01-26 21:41     ` Andy Shevchenko
2021-01-27 17:57       ` Yury Norov
2021-01-27  8:20     ` Paul Gortmaker
2021-01-26 17:11 ` [PATCH 7/8] lib: test_bitmap: add tests for "N" alias Paul Gortmaker
2021-01-26 17:11 ` [PATCH 8/8] rcu: deprecate "all" option to rcu_nocbs= Paul Gortmaker
2021-01-26 21:36   ` Yury Norov
2021-01-26 22:17     ` Paul E. McKenney
2021-01-26 22:27 ` [PATCH v3 0/8] support for bitmap (and hence CPU) list "N" abbreviation Yury Norov
2021-01-27  9:12   ` Paul Gortmaker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAAH8bW9pQ_==uF3_Zo6K-ijqHXDVcW_-9gx0fvFWXe=fEvsg9A@mail.gmail.com' \
    --to=yury.norov@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=fweisbec@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=lizefan@huawei.com \
    --cc=mingo@kernel.org \
    --cc=paul.gortmaker@windriver.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH 5/8] lib: bitmap_getnum: separate arg into region and field' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).