[Freeipa-devel] [PATCHES] 206-209 Add default CFLAGS & fix hardened build

[Petr Spacek]

On 6.12.2013 11:52, Jan Cholasta wrote:
> freeipa-jcholast-208.1-Add-stricter-default-CFLAGS-to-Makefile.patch
>
>
>>From 85ad15d522274a711c87f92ed91889b781d7455e Mon Sep 17 00:00:00 2001
> From: Jan Cholasta<jcholast at redhat.com>
> Date: Wed, 4 Dec 2013 18:42:36 +0100
> Subject: [PATCH 3/5] Add stricter default CFLAGS to Makefile.
>
> https://fedorahosted.org/freeipa/ticket/3896
> ---
>   Makefile | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 0664ddd..a722634 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -52,6 +52,9 @@ endif
>
>   PYTHON ?= $(shell rpm -E %__python || echo /usr/bin/python)
>
> +CFLAGS := -g -O2 -Werror -Wall -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers $(CFLAGS)
> +export CFLAGS

I don't like -Wno-sign-compare -Wno-missing-field-initializers parameters, 
both could find some nasty surprises in the code.

Also, I would add those:

-Wformat-nonliteral
         If -Wformat is specified, also warn if the format string is not
         a string literal and so cannot be checked, unless the format
         function takes its format arguments as a |va_list|.

-Winit-self
     Warn about uninitialized variables that are initialized with
     themselves.

-Wshadow
     Warn whenever a local variable or type declaration shadows another
     variable, parameter, type, or class member (in C++), or whenever a
     built-in function is shadowed.

-Wpointer-arith
     Warn about anything that depends on the “size of” a function type or
     of |void|.

-Wbad-function-cast
     Warn whenever a function call is cast to a non-matching type.

-Wjump-misses-init
     Warn if a |goto| statement or a |switch| statement jumps forward
     across the initialization of a variable, or jumps backward to a
     label after the variable has been initialized. This only warns about
     variables that are initialized when they are declared.
I have seen bugs like this in bind-dyndb-ldap recently.


Little bit more controversial options are:

-Wswitch-enum
     Warn whenever a |switch| statement has an index of enumerated type
     and lacks a |case| for one or more of the named codes of that
     enumeration. |case| labels outside the enumeration range also
     provoke warnings when this option is used. The only difference
     between -Wswitch and this option is that this option gives a warning
     about an omitted enumeration code even if there is a |default| label.
IMHO default in case should (usually) catch only 'garbage' (corrupted memory 
etc.) and all expected values should be specified explicitly by 'case' 
statements. Of course, this doesn't work in all cases ...

A less invasive alternative is:
-Wswitch-default
     Warn whenever a |switch| statement does not have a |default| case.

-Wconversion
     Warn for implicit conversions that may alter a value. [...]
This can produce a lot of warnings because of unsigned int <-> size_t 
conversions. Unfortunately, I didn't find a way how to disable this warning 
only for size_t conversions.

-Wundef
     Warn if an undefined identifier is evaluated in an ‘#if’ directive.
We have #ifdef for that :-)

-Wcast-qual
     Warn whenever a pointer is cast so as to remove a type qualifier
     from the target type. For example, warn if a |const char *| is cast
     to an ordinary |char *|.
I don't insist on this. Sometimes, we are lazy and just cast the type and hope 
that the called function really does not modify the value ...

-Wwrite-strings
     When compiling C, give string constants the type |const
     char[|length|]| so that copying the address of one into a
     non-|const| |char *| pointer produces a warning.
This would be interesting experiment or some long-term ticket.

-- 
Petr^2 Spacek