All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Alexey Gladkov <gladkov.alexey@gmail.com>, linux-sparse@vger.kernel.org
Subject: Re: [PATCH] Add symantic index utility
Date: Tue, 10 Mar 2020 18:12:02 +0100	[thread overview]
Message-ID: <20200310171202.y5rhsydmmbewoarm@ltop.local> (raw)
In-Reply-To: <20200310150713.GB19012@redhat.com>

On Tue, Mar 10, 2020 at 04:07:14PM +0100, Oleg Nesterov wrote:
> 
> Annoyingly, this triggers a lot of sparse_error's in pre-process.c:collect_arg().
> And just in case, of course this is not specific to dissect/sindex, ./sparse or
> anything else will equally complain.
> 
> For example,
> 
>   1011  static inline bool page_expected_state(struct page *page,
>   1012                                          unsigned long check_flags)
>   1013  {
>   1014          if (unlikely(atomic_read(&page->_mapcount) != -1))
>   1015                  return false;
>   1016
>   1017          if (unlikely((unsigned long)page->mapping |
>   1018                          page_ref_count(page) |
>   1019  #ifdef CONFIG_MEMCG
>   1020                          (unsigned long)page->mem_cgroup |
>   1021  #endif
>   1022                          (page->flags & check_flags)))
>   1023                  return false;
>   1024
>   1025          return true;
>   1026  }
> 
> leads to
> 
> 	mm/page_alloc.c:1019:1: error: directive in macro's argument list
> 	mm/page_alloc.c:1021:1: error: directive in macro's argument list
> 
> and it is not immediately clear why. Yes, because "unlikely" is a macro.
> 
> Can't we simply remove this sparse_error() ? "#if" inside the macro's args
> is widely used in kernel, gcc doesn't complain, afaics pre-process.c handles
> this case correctly.

I'm quite reluctant to simply suppress it.
My (contradictory) point of view is that it is because it's not
immediately clear there is a problem that the warning is needed
but, OTOH, people and the Standard, want to use macros transparently
so a macro wrapping a function call should behave just like directly
calling the function. And yes both Sparse and GCC seem to be able
to handle this, so it's maybe only a restriction for more primtive
preprocessors. I dunno.

Some arguments/justifications for the arning can be found at:
	https://www.spinics.net/lists/kernel/msg1636994.html

Anyway, only a warning should be issued (I'll send a patch for this).
I also wouldn't mind to add a new warning flag to suppress it,
something like -Wno-directive-within-macro.

-- Luc

  reply	other threads:[~2020-03-10 17:12 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09 15:25 [PATCH] Add symantic index utility Alexey Gladkov
2020-03-09 22:37 ` Luc Van Oostenryck
2020-03-10 15:07   ` Oleg Nesterov
2020-03-10 17:12     ` Luc Van Oostenryck [this message]
2020-03-11 12:04       ` Oleg Nesterov
2020-03-11 16:47         ` Luc Van Oostenryck
2020-03-11  9:07     ` Dan Carpenter
2020-03-11 11:33       ` Oleg Nesterov
2020-03-11 17:11         ` Luc Van Oostenryck
2020-03-11 17:06       ` Luc Van Oostenryck

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=20200310171202.y5rhsydmmbewoarm@ltop.local \
    --to=luc.vanoostenryck@gmail.com \
    --cc=gladkov.alexey@gmail.com \
    --cc=linux-sparse@vger.kernel.org \
    --cc=oleg@redhat.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.