Re: Getting rid of warnings

Discussion in 'C Programming' started by Markus Wichmann, Mar 6, 2012.

  1. On 05.03.2012 10:33, Guillaume Dargaud wrote:
    > Hello all,
    > I usually try to get my code clean enough so that no warnings remain at
    > compile time. So I'm annoyed by the 2 following warnings which I can't get
    > rid of. Suggestions (in the form of code rewrite or #pragma) are welcome.
    >
    > Exemple 1:
    > #define MAX_TRACES 100
    > int NbTraces=0; // Set somewhere else in the program
    >
    > void F(void) {
    > int i, Idx[MAX_TRACES];
    > for (j=0; j<MAX_TRACES; j++) Idx[j]=0; // Doesn't change anything
    > for (j=0; j<MAX_TRACES; j++) GetSomething(&Idx[j]);
    > for (j=0; j<NbTraces; j++) i = Idx[j];
    > }
    >
    > The last line gives me:
    > Warning: Local 'Idx' might not have been fully initialized.
    > Obviously NbTraces<MAX_TRACES and anyway I fill up the Idx array with 0, so
    > what is it complaining about ?
    >


    Well, /you/ know that NbTraces will never exceed MAX_TRACES, but the
    compiler doesn't. Perhaps you should insert an

    assert(NbTraces < MAX_TRACES);

    before the loops.

    And, by the way, why don't you just replace that last line with

    i = Idx[NbTraces - 1];

    >
    > Example 2:
    > #define ADD(Frmt) { \
    > if (NULL==strchr(Frmt, '%')) sprintf(DimName, "%s/" Frmt, Server.Name);\
    > else sprintf(DimName, "%s/" Frmt, Server.Name,
    > Port);
    >
    > ADD("Something");
    > ADD("SomePort%d");
    >
    > The first add gives me:
    > DimPpcServer.c:1126: warning: too many arguments for format
    > And the 2nd one:
    > DimPpcServer.c:1130: warning: too few arguments for format
    > I understand the reason (the expansion of "%s/Something" on the 2nd line of
    > the macro has an extra argument).
    >
    > Thanks


    Perhaps it would be better to replace that very kludgy macro with a
    function like

    #ifdef __GNUC__
    __attribute__((format(printf, 1, 2)))
    #endif
    int add(const char* fmt, ...)
    {
    size_t l = strlen(Server.Name);
    va_args ap;
    int r;
    va_start(ap, fmt);
    memcpy(DimName, Server.Name, l);
    DimName[l] = '/';
    r = vsprintf(DimName + l + 1, fmt, ap);
    va_end(ap);
    return r;
    }

    The first three lines give you format string parameter checking on GCC
    (I don't know how to do that on other compilers) for this function.

    However, you should really apply some length checking here, the code is
    vulnerable to buffer overflows on DimName.

    If your toolchain supports the C99 inline keyword, you can define this
    function in a header file as inline. You should then add another source
    file to your project, containing just the inclusion of the header file
    with this function and

    extern int add(const char* fmt, ...);

    That should be just as efficient as a macro expansion and safer than
    that (you know, side-effect wise).

    HTH,
    Markus
    Markus Wichmann, Mar 6, 2012
    #1
    1. Advertising

  2. Markus Wichmann

    Jens Gustedt Guest

    Am 03/06/2012 01:44 PM, schrieb Guillaume Dargaud:
    > Markus Wichmann wrote:
    >
    >> And, by the way, why don't you just replace that last line with
    >> i = Idx[NbTraces - 1];

    >
    > Actually the loop does other things which I left out.
    >
    >
    > Ike Naar wrote:
    >
    >> Does it help if you replace the two lines above by
    >> int i, Idx[MAX_TRACES] = {0};

    >
    > Yes it does go away and I'm surprised ! I thought I would have to set
    > {0,0,...} but since I didn't know the value of MAX_TRACES, that wasn't
    > possible.
    > Does this syntax means that only the 1st element is set to 0 or all of them?


    No, all elements that are not explicitly mentioned in an initializer are
    always initialized by 0

    In fact the above initializer { 0 } works for any type of array (and
    other variable) as long as MAX_TRACES is a compile time integer constant.

    Jens
    Jens Gustedt, Mar 6, 2012
    #2
    1. Advertising

  3. Markus Wichmann

    James Kuyper Guest

    On 03/06/2012 07:44 AM, Guillaume Dargaud wrote:
    > Markus Wichmann wrote:

    ....
    >> Does it help if you replace the two lines above by
    >> int i, Idx[MAX_TRACES] = {0};

    >
    > Yes it does go away and I'm surprised ! I thought I would have to set
    > {0,0,...} but since I didn't know the value of MAX_TRACES, that wasn't
    > possible.
    > Does this syntax means that only the 1st element is set to 0 or all of them?


    It happens to set all of them to 0, but for two different reasons. It's
    easier to explain those two reasons if I change it a bit:

    int Idx[MAX_TRACES] = {1};

    That code explicitly sets Idx[0] to 1. However, because an explicit
    initializer for par of the array has been provided, all of the elements
    other than the one specified get implicitly zero-initialized. The same
    rule applies to structures - if an incomplete initializer is provided,
    the members not explicitly initialized are implicitly zero-initialized.
    --
    James Kuyper
    James Kuyper, Mar 6, 2012
    #3
  4. Markus Wichmann

    Eric Sosman Guest

    On 3/6/2012 7:44 AM, Guillaume Dargaud wrote:
    > [...]
    > Actually the loop does other things which I left out.


    In other words, the warning was not for the code you posted,
    but for some other code we can only guess at. Oh, joy.

    > [...]
    > here I simplified those
    > macros which are a lot more complex and can't actually be easily replaced by
    > functions (lots of ## and # expansions which I left out).


    The problem with the macro as posted is clear (of course, it
    may be unrelated to the actual program). After expansion of
    `ADD("Something")' you have

    if (NULL==strchr("Something", '%'))
    sprintf(DimName, "%s/Something", Server.Name);
    else
    sprintf(DimName, "%s/Something", Server.Name, Port);

    After expanding `ADD("SomePort/%d")' you have

    if (NULL==strchr("SomePort/%d", '%'))
    sprintf(DimName, "%s/SomePort/%d", Server.Name);
    else
    sprintf(DimName, "%s/SomePort/%d", Server.Name, Port);

    In each case, one of the two sprintf() calls draws a warning,
    and the warning makes sense because the format does not match the
    arguments. You happen to know that the incorrect call will not
    execute, but the compiler does not: It has not evaluated strchr()
    and performed the NULL comparison during compilation, but has left
    them for run-time.

    Since you haven't shown actual code I can't suggest a way
    to improve matters, except for the generic "Don't Do That."

    --
    Eric Sosman
    d
    Eric Sosman, Mar 6, 2012
    #4
  5. Jens Gustedt <> writes:

    > Am 03/06/2012 01:44 PM, schrieb Guillaume Dargaud:

    <sni>
    >>> Does it help if you replace the two lines above by
    >>> int i, Idx[MAX_TRACES] = {0};

    >>
    >> Yes it does go away and I'm surprised ! I thought I would have to set
    >> {0,0,...} but since I didn't know the value of MAX_TRACES, that wasn't
    >> possible.
    >> Does this syntax means that only the 1st element is set to 0 or all of them?

    <snip>
    > In fact the above initializer { 0 } works for any type of array (and
    > other variable) as long as MAX_TRACES is a compile time integer
    > constant.


    I think it works for variably modified array types as well. In other
    word, MAX_TRACES could be (or could expand to) an expression that is not
    a compile-time constant.

    --
    Ben.
    Ben Bacarisse, Mar 6, 2012
    #5
  6. Markus Wichmann

    Jens Gustedt Guest

    Am 03/06/2012 06:17 PM, schrieb Ben Bacarisse:
    > Jens Gustedt <> writes:
    >> In fact the above initializer { 0 } works for any type of array (and
    >> other variable) as long as MAX_TRACES is a compile time integer
    >> constant.

    >
    > I think it works for variably modified array types as well. In other
    > word, MAX_TRACES could be (or could expand to) an expression that is not
    > a compile-time constant.


    No, for VLA initializers are not allowed, unfortunately.

    Jens
    Jens Gustedt, Mar 6, 2012
    #6
  7. Jens Gustedt <> writes:

    > Am 03/06/2012 06:17 PM, schrieb Ben Bacarisse:
    >> Jens Gustedt <> writes:
    >>> In fact the above initializer { 0 } works for any type of array (and
    >>> other variable) as long as MAX_TRACES is a compile time integer
    >>> constant.

    >>
    >> I think it works for variably modified array types as well. In other
    >> word, MAX_TRACES could be (or could expand to) an expression that is not
    >> a compile-time constant.

    >
    > No, for VLA initializers are not allowed, unfortunately.


    Ah, yes, so I see (now). Thanks for that.

    --
    Ben.
    Ben Bacarisse, Mar 6, 2012
    #7
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    7
    Views:
    10,010
    dabooka
    Oct 7, 2008
  2. glen stark
    Replies:
    3
    Views:
    348
  3. Ted Sung
    Replies:
    1
    Views:
    308
    Sherm Pendley
    Aug 30, 2004
  4. Replies:
    3
    Views:
    156
  5. Tomasz Chmielewski
    Replies:
    12
    Views:
    168
Loading...

Share This Page