[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

On 6.12.2013 11:52, Jan Cholasta wrote:

From 85ad15d522274a711c87f92ed91889b781d7455e Mon Sep 17 00:00:00 2001
From: Jan Cholasta<jcholast redhat com>
Date: Wed, 4 Dec 2013 18:42:36 +0100
Subject: [PATCH 3/5] Add stricter default CFLAGS to Makefile.

  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:

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

    Warn about uninitialized variables that are initialized with

    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.

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

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

    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:

    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:
    Warn whenever a |switch| statement does not have a |default| case.

    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.

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

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

    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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]