On Fri 23-02-18 00:56:26, Stephen Rothwell wrote:
> Hi Michal,
> On Thu, 22 Feb 2018 08:11:00 +0100 Michal Hocko <mho...@kernel.org> wrote:
> >
> > This is interesting. I thought that IS_ENABLED(CONFIG_HAVE_MEMBLOCK)
> > would have the same meaning as ifdef CONFIG_HAVE_MEMBLOCK so the branch
> > will never be considered. If that is not the case then I would rather
> > reintroduce that ifdef. We already have those in the function anyway.
> Actually, you don't need a definition of memblock_next_valid_pfn() in the
> !CONFIG_HAVE_MEMBLOCK case, just a declaration, so the minimal fix is
> to move the declaration out of the #ifdef CONFIG_HAVE_MEMBLOCK in the
> header file.

You are right.

> That way if there is any use of memblock_next_valid_pfn()
> introduced that is no guarded by IS_ENABLED(CONFIG_HAVE_MEMBLOCK) the
> build will fail to link.  I like IS_ENABLED() being used wherever
> possible because it allows us better compiler coverage (in the face of
> CONFIG options) even if the compiler then elides the actual code.  It
> also breaks the code up less than #ifdef's.
> Your choice, of course.

The function already has those ugly ifdefs so I would keep it
consistent. Deuglyfying it would need a bigger stick.
Michal Hocko

Reply via email to