Changes between Version 38 and Version 39 of Guidelines/MaintenanceGuidelines

Timestamp:

Feb 3, 2010, 8:17:14 PM (13 years ago)

Author:

phorgan1

Comment:

Started adding information about gcc warnings

Guidelines/MaintenanceGuidelines

@@ -903 +903 @@
 More information needed here!
 
-||? || class does not have a  virtual destructor||Suppress or provide a virtual destructor.||??||
+== Individual Warnings ==
+'''comparison between signed and unsigned integer expressions'''
+
+    '''-Wsign-compare''' - enable the warning (also turned on by -Wextra)[[BR]]
+    '''-Wno-sign-compare''' - disable the warning[[BR]]
+
+    Almost always points to a real issue. The ranges of these are obviously
+    different, for example signed char -128 - 127 and unsigned char 0 - 255 for
+
+                    range               bit pattern
+    signed char     -128 to 127     01111111 to 10000000
+    unsigned char    0 to 255       00000000 to 11111111
+
+    That means: signed char     unsigned char
+      0 to 127      +               +
+    128 to 255      -               +
+
+    In the range from 128 to 255 a comparison from one to the other makes no
+    sense.  They can have the same bit pattern, yet test as not equal.  This
+    is the source of many subtle and hard to find bugs.
+
+    The same problem can occur with all of the integral types. For example with
+    16-bit short int unsigned ranges from -32768 to 32767, and the signed short
+    ranges from 0 to 65535.
+
+    To fix, if you are sure that all the values for both things being compared
+    fall into the overlap where their values are the same, you can static_cast
+    one type to the other, if you must, but if you can change one of the types
+    it's a better solution.  If you are sure that you can fit all values into
+    the compared range, i.e. positive numbers from 
+    0 to (1<<(sizeof(IntegralType)*8-1))-1, it would make more sense to
+    declare the signed one as an unsigned equivalent, (perhaps size_t).  In
+    general, if you don't intend for a variable to ever have negative values,
+    take advantage of the diagnostic capabilities of the compiler and don't
+    declare it as an unsigned type.  After you've done this, if you see the
+    warning again for one of these same variables, it is warning you of this
+    same danger again.
+
+    This often shows up with loop counters.  We all learned to use int, but
+    if it's truly counting only positive numbers, size_t is better.
+
+'''missing braces around initializer for ‘main()::X'''
+
+    '''-Wmissing-braces''' - enable the warning (also turned on by -Wall)[[BR]]
+    '''-Wno-missing-braces''' - disable the warning
+
+    This warning is trying to let you know that what you think you're
+    initializing may not actually be what you're initializing.
+
+
+{{{
+        struct X { int i; int j;};
+        struct Y { struct X x; int j; };
+
+        Y y={1,2};
+        return 0;
+
+}}}
+
+    The above example initializes both the elements of X in Y, but doesn't
+    initialize j.  Change to:
+
+        {{{Y y={{1},2};}}}
+
+    to initialize half of X and all of Y, or better something like:
+
+        {{{Y y={{1,2},3};}}}
+
+    The same kind of problem can come up with:
+
+        {{{int a[2][2]={ 0, 1, 2 };}}}
+
+    versus:
+
+        {{{int a[2][2]={ {0,1},2};}}} or {{{int a[2][2]={ 0, {1,2}};}}}
+
+'''deprecated conversion from string constant to ‘char*’'''
+    '''-Wwrite-strings''' - enable this warning (disabled by default for C and enabled by default for C++)
+    '''-Wno-write-strings''' - disable this warning
+
+        For C will warn about trying to modify string constants, but only if defined const.
+        For C++ often symptomatic of a real bug since string literals are not writeable in C++.
+
+        This one shows up when you have a string literal, like "Hello world!" which
+        is arguably const data, and you assign it no a non-const char* either
+        directly or via passing as an argument to a functions which expects a non-const char *.
+        both:
+            {{{
+               char* cp="Hello world!";
+            }}}
+        and
+            {{{
+                void foo(char *cp);
+                foo("Hello world")
+            }}}
+        will get this warning because you're using a string literal where a char*
+        is expected.  If you say, so what?  See if you can predict what will print
+        in the example below.
+
+        {{{
+        #include <iostream>
+
+        void
+        foo(char *cp)
+        {
+            cp[0]='W';
+        }
+
+        int main()
+        {
+                char *str="Hello, world";
+                foo(str);
+                std::cout << str << std::endl;
+        }
+        }}}
+
+        If you said that Wello, world will print, think again.  Your most likely
+        output is something like:
+                '''Segmentation fault (core dumped)'''
+        A string literal is put into a section that is read only and trying to
+        write to it makes things blow up!
+
+        The right fix is to make anything that a string literal can be assigned
+        to a const char*.
+        Unfortunately, sometimes you can't fix the problem at the source, because
+        it's someone else's code, and you can't change the declaration.  For
+        example, if you had this function:
+
+        {{{
+        int PyArg_ParseTupleAndKeywords(PyObject *arg, PyObject *kwdict,
+                                char *format, char **kwlist, ...);
+        }}}
+
+    and you had a keyword list to pass it like this:
+
+        {{{
+        static char *kwlist[] = {"fget", "fset", "fdel", "doc", 0};
+                if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOOO:property",
+                                          kwlist, &get, &set, &del, &doc))
+        }}}
+
+        you would get the warning on the kwlist assignment, but when you change
+        it to:
+
+        {{{
+        static const char *kwlist[] = {"fget", "fset", "fdel", "doc", 0};
+        }}}
+
+        you'll get:
+
+            error: invalid conversion from ‘const char**’ to ‘char**’
+
+        unless you change the call to:
+
+        {{{
+        if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOOO:property",
+                                          const_cast<char **>(kwlist), &get, &set, &del, &doc))
+        }}}
+
+        annoyingly having to incorrectly cast away the constness of kwlist.
+        Now the code is arguably correct and it wasn't before.
+
+ |? || class does not have a  virtual destructor||Suppress or provide a virtual destructor.||??||
 
 '''Intel'''