Re: [patch] hugetlb: remove dummy definitions of HPAGE_MASK andHPAGE_SIZE

[David Rientjes]

On Thu, 17 Nov 2011, David Daney wrote:

> A counter argument would be:
> 
> There are hundreds of places in the kernel where dummy definitions are
> selected by !CONFIG_* so that we can do:
> 
>    if (test_something()) {
>       do_one_thing();
>    } else {
>       do_the_other_thing();
>    }
> 
> 
> Rather than:
> 
> #ifdef CONFIG_SOMETHING
>    if (test_something()) {
>       do_one_thing();
>    } else
> #else
>    {
>       do_the_other_thing();
>    }
> 
> 
> 
> We even do this all over the place with dummy definitions selected by
> CONFIG_HUGETLB_PAGE, What exactly makes HPAGE_MASK special and not the
> hundreds of other similar situations?
> 

Dummy functions that return 0 when a feature isn't enabled isn't the 
problem, that's very convenient.  Definitions of constants are a 
completely separate story because they can be easily used outside the 
required context and cause brekage.  What happens if you reference a 
variable in a function that is declarated in a different function?  Does 
the compiler put a BUG() in there and let you explode at runtime?  This is 
just absurd, every other arch has done the necessary declarations in their 
own header files and everything works fine because there's a clear config 
dependency on using HPAGE_*.  Adding dummy definitions to panic at runtime 
is irresponsibly to an extreme.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/