Re: [PATCH v5 1/2] string: _FORTIFY_SOURCE=3 using __builtin_dynamic_object_size

[Paul Eggert <eggert@cs.ucla.edu>]

On 12/16/20 7:24 AM, Siddhesh Poyarekar via Libc-alpha wrote:

> +# elif _FORTIFY_SOURCE > 3 && __glibc_clang_prereq(9, 0)
> +#  warning _FORTIFY_SOURCE > 3 unsupported falling back to 3
> +#  define __USE_FORTIFY_LEVEL 3
> +# elif _FORTIFY_SOURCE > 3
> +#  warning _FORTIFY_SOURCE > 3 unsupported falling back to 2

That'd be a strange warning to get, no?

> +#  define __USE_FORTIFY_LEVEL 2
> +# elif _FORTIFY_SOURCE > 2 && !__glibc_clang_prereq(9, 0)
> +#  warning _FORTIFY_SOURCE=3 requires LLVM 9.0 or later, falling back to 2
> +#  define __USE_FORTIFY_LEVEL 2
> +# elif _FORTIFY_SOURCE > 2
> +#  define __USE_FORTIFY_LEVEL 3
>  # elif _FORTIFY_SOURCE > 1
>  #  define __USE_FORTIFY_LEVEL 2

This code is in an unusual order 3, 2, 2, 3, 2 for __USE_FORTIFY_LEVEL, 
which makes it hard to follow.

Also, the warning shouldn't imply that one should use LLVM 9.0 or later. 
It's not our job to recommend LLVM versions, plus that warning's wording 
would likely become dated anyway.

Also, there should be spaces before the parens.

How about something like the following instead?  It uses simpler and more 
future-proof warnings, and I find it easier to follow:

   +# elif _FORTIFY_SOURCE > 2 && __glibc_clang_prereq (9, 0)
   +#  if _FORTIFY_SOURCE > 3
   +#   warning _FORTIFY_SOURCE > 3 is treated like 3 on this platform
   +#  endif
   +#  define __USE_FORTIFY_LEVEL 3
    # elif _FORTIFY_SOURCE > 1
   +#  if _FORTIFY_SOURCE > 2
   +#   warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform
   +#  endif
    #  define __USE_FORTIFY_LEVEL 2