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.  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.

-- 
Cheers,
Stephen Rothwell

Attachment: pgpWuZy3GUkY5.pgp
Description: OpenPGP digital signature

Reply via email to