Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #18220 > unrolled thread

Getting rid of warnings

Back to article view | Back to comp.lang.c

Contents

  Getting rid of warnings Guillaume Dargaud <use_the_contact_form@www.gdargaud.net> - 2012-03-05 10:33 +0100
    Re: Getting rid of warnings Ike Naar <ike@iceland.freeshell.org> - 2012-03-05 11:16 +0000
    Re: Getting rid of warnings Barry Schwarz <schwarz45@yahoo.com> - 2012-03-05 13:22 -0800
    Re: Getting rid of warnings Markus Wichmann <nullplan@gmx.net> - 2012-03-06 09:12 +0100
      Re: Getting rid of warnings Guillaume Dargaud <use_the_contact_form@www.gdargaud.net> - 2012-03-06 13:44 +0100
        Re: Getting rid of warnings Jens Gustedt <jens.gustedt@loria.fr> - 2012-03-06 14:02 +0100
          Re: Getting rid of warnings Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-03-06 17:17 +0000
            Re: Getting rid of warnings Jens Gustedt <jens.gustedt@loria.fr> - 2012-03-06 22:43 +0100
              Re: Getting rid of warnings Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-03-06 22:49 +0000
        Re: Getting rid of warnings James Kuyper <jameskuyper@verizon.net> - 2012-03-06 08:04 -0500
          Re: Getting rid of warnings Guillaume Dargaud <use_the_contact_form@www.gdargaud.net> - 2012-03-13 10:59 +0100
        Re: Getting rid of warnings Eric Sosman <esosman@ieee-dot-org.invalid> - 2012-03-06 08:27 -0500

#18220 — Getting rid of warnings

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 ?


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
-- 
Guillaume Dargaud
http://www.gdargaud.net/

[toc] | [next] | [standalone]

#18223

On 2012-03-05, Guillaume Dargaud <use_the_contact_form@www.gdargaud.net> 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

Does it help if you replace the two lines above by

  int i, Idx[MAX_TRACES] = {0};

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

This is ugly, but it will probably work:

  #define ADD0(Frmt) sprintf(DimName, "%s/" Frmt, Server.Name);
  #define ADD1(Frmt) sprintf(DimName, "%s/" Frmt, Server.Name, Port);

  ADD0("Something");
  ADD1("SomePort%d");

[toc] | [prev] | [next] | [standalone]

#18248

On Mar 5, 3:33 am, Guillaume Dargaud
<use_the_contact_f...@www.gdargaud.net> 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 ?

There are times when the compiler is required to issue a diagnostic.
Unfortunately, there is no restriction against extraneous or
superfluous diagnostics, such as this.  Many systems have options that
allow you to supress undesired diagnostics.  The documentation for
your system should tell you if it has that option and how to invoke
it.

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

After substituion, your format string has only ONE conversion
specifier while there are TWO arguments to be formatted in the in the
second call to sprintf.  While not a problem from a language
perspective, this diagnostic is not unreasonable.  See previous
comment about possibly supressing it.

> And the 2nd one:
> DimPpcServer.c:1130: warning: too few arguments for format

After substition, your format string has TWO conversion specifiers BUT
the first call to sprintf has only ONE argument to be converted.  This
is a serious programming error which immediately invokes undefined
behavior.  While the compiler is not required to diagnose this
situation, you should consider yourself fortunate that it did.

> I understand the reason (the expansion of "%s/Something" on the 2nd line of
> the macro has an extra argument).

Apparently your understanding is incomplete.

[toc] | [prev] | [next] | [standalone]

#18257

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

[toc] | [prev] | [next] | [standalone]

#18261

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?

>   #define ADD0(Frmt) sprintf(DimName, "%s/" Frmt, Server.Name);
>   #define ADD1(Frmt) sprintf(DimName, "%s/" Frmt, Server.Name, Port);

Yeah, I guess it's probably the simplest solution. 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).
-- 
Guillaume Dargaud
http://www.gdargaud.net/

[toc] | [prev] | [next] | [standalone]

#18262

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

[toc] | [prev] | [next] | [standalone]

#18268

Jens Gustedt <jens.gustedt@loria.fr> 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.

[toc] | [prev] | [next] | [standalone]

#18276

Am 03/06/2012 06:17 PM, schrieb Ben Bacarisse:
> Jens Gustedt <jens.gustedt@loria.fr> 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

[toc] | [prev] | [next] | [standalone]

#18277

Jens Gustedt <jens.gustedt@loria.fr> writes:

> Am 03/06/2012 06:17 PM, schrieb Ben Bacarisse:
>> Jens Gustedt <jens.gustedt@loria.fr> 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.

[toc] | [prev] | [next] | [standalone]

#18263

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

[toc] | [prev] | [next] | [standalone]

#18659

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

That's new to me.
Thanks.
-- 
Guillaume Dargaud
http://www.gdargaud.net/

[toc] | [prev] | [next] | [standalone]

#18264

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
esosman@ieee-dot-org.invalid

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web