Summary: Handle unused arguments
Project: lwIP - A Lightweight TCP/IP stack
Submitted by: goldsimon
Submitted on: Montag 26.03.2007 um 16:03
Priority: 5 - Normal
Assigned to: None
Discussion Lock: Any
In some functions unused arguments are used (e.g. "arg;" or "arg = arg;") to
prevent compiler warnings. This should either be modified to something like
"LWIP_UNUSED_ARG(arg);" or completely left away.
It's not that I fear these statements would bloat lwIP binary size, only that
different compilers have very different views of what to warn about.
Statements like "arg = arg;" or "arg;" often give a warning like "statement
has no effect". Personally, I prefer simply not to warn about unused
I would define LWIP_UNUSED_ARG to nothing by default and let people override
it if they really need it.
That patch seems rather complete, but I want to look over all the code again:
as Kieran said: The best solution would be to remove any unused arguments from
the function declaration!
In my opinion, cc.h is a good location since it _is_ port specific. Some
compilers complain like "statement has no effect" when they find something
like "(void)arg;" (although Paradigm and gcc don't seem among them, at least
with your compile time options :)
About remove any unsed arguments, it will not be possible always. By example,
in api_msg.c, in recv_udp, the prototype is defined by udp_recv, so, you can't
change it without breaking the api.
Why not about arch.h? It already give default define for PACK_STRUCT_BEGIN
and others, and it also include cc.h. So, if this default is not good for
your compiler, you can provide your own define in your cc.h (in fact, it's
more compiler than port dependant).
This default is good for me, if it's good for gcc.
Forgot to mention that my patch was tested with all "Inefficient Coding"
warnings turned on in Paradigm Pro, particularly, "Parameter 'ident is never
used" and "'ident declared but never used."
Regarding to unused argument removal, I think that's not that easy without
breaking existing APIs. For example, ip_timer() utilises sys_timeout()
interface, which requires a timeout handler with void argument.
"-Wuninitialized -Winit-self" should do all the 'arg = arg' cases.
For 'if(arg);' Supposedly -Wextra warns when:
' An empty body occurs in an `if' or `else' statement.'
It also warns about a lot of other things too so may well be quite noisy. I
guess you can search in the compiler output for the specific warning message
that gets produced for that case.
BTW, I hadn't noticed this patch discussion before. Out of interest, in our
code we use the following which from experience on various compilers is more
portable at preventing warnings - some compilers are quite aggressive at
reporting them. It's slightly harder to apply retrospectively because you
need the type though:
Furtunately, gcc does not warn if you cast the unused variable to (void).
Either most of the users use gcc or many other compilers do it like this (or
don't warn at all?). But casting to void (as it is now) was in the code
already. This 'patch' only moves the casting into a define, which gives us a)
cleaner code and b) the possibility to adopt to other compilers.
Unfortunately, type information is not included...
The downside to your approach is that the statement (if not optimized away)
generates code, while '(void)arg;' doesn't (even with '-O0' for gcc).
If you want to change it, though, the places to change are now easy to find
Closing this as done, as I didn't find more appearences.