diff mbox

Android: disable unused parameter warnings

Message ID 20140415002727.A4C7313FC9D@ushik.mtv.corp.google.com
State Accepted
Headers show

Commit Message

Greg Hackmann April 15, 2014, 12:06 a.m. UTC
Change-Id: I721b4a7de62a569d272fa31dfe6f3bb66192984a
Signed-off-by: Greg Hackmann <ghackmann@google.com>
---
 hostapd/Android.mk        | 3 +++
 wpa_supplicant/Android.mk | 3 +++
 2 files changed, 6 insertions(+)

Comments

Kalle Valo April 15, 2014, 5:16 a.m. UTC | #1
Greg Hackmann <ghackmann@google.com> writes:

> Change-Id: I721b4a7de62a569d272fa31dfe6f3bb66192984a
> Signed-off-by: Greg Hackmann <ghackmann@google.com>
> ---
>  hostapd/Android.mk        | 3 +++
>  wpa_supplicant/Android.mk | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/hostapd/Android.mk b/hostapd/Android.mk
> index 6db82f8..74c9b27 100644
> --- a/hostapd/Android.mk
> +++ b/hostapd/Android.mk
> @@ -24,6 +24,9 @@ L_CFLAGS += -DVERSION_STR_POSTFIX=\"-$(PLATFORM_VERSION)\"
>  # Set Android log name
>  L_CFLAGS += -DANDROID_LOG_NAME=\"hostapd\"
>  
> +# Disable unused parameter warnings
> +L_CFLAGS += -Wno-unused-parameter

Wouldn't it be better to fix the actual warnings?
Jouni Malinen April 17, 2014, 2:29 p.m. UTC | #2
On Tue, Apr 15, 2014 at 08:16:32AM +0300, Kalle Valo wrote:
> Greg Hackmann <ghackmann@google.com> writes:
> > diff --git a/hostapd/Android.mk b/hostapd/Android.mk
> > @@ -24,6 +24,9 @@ L_CFLAGS += -DVERSION_STR_POSTFIX=\"-$(PLATFORM_VERSION)\"
> > +# Disable unused parameter warnings
> > +L_CFLAGS += -Wno-unused-parameter
> 
> Wouldn't it be better to fix the actual warnings?

Depends on what you mean by fixing them.. I did not even know that
someone would compile with -Wunused-parameter (or well, maybe more
likely with -Wextra without disabling this specific warning). This
results in 3522 "unused parameter" warning with my build configuration.

I'm using "-Wextra -Wno-unused-parameter" for a good reason of getting
rid of these bogus warnings. It is not an error, or worth a warning,
IMHO, if a function does not use every single parameter. There is a
large number of function pointers in wpa_supplicant/hostapd for
implementing generic interfaces like EAP method operations or
configuration parsers. Some EAP methods may need all the parameters,
many don't. Trying to somehow encode every such location with
__attribute__((unused)) to avoid this warning sounds both useless work
and ugly in the end.
Holger Schurig April 17, 2014, 2:46 p.m. UTC | #3
Or mark them explictly:

some_eap_meth(eap *moo; param *boo)
{
  (void)boo;

  if (moo->xxx)
    barf();
}

But i'd say annotating all the existing 3522 places with those things
is a job for a bureaucrat!  :-)
Dmitry Shmidt April 21, 2014, 7:16 p.m. UTC | #4
On Thu, Apr 17, 2014 at 7:29 AM, Jouni Malinen <j@w1.fi> wrote:
> On Tue, Apr 15, 2014 at 08:16:32AM +0300, Kalle Valo wrote:
>> Greg Hackmann <ghackmann@google.com> writes:
>> > diff --git a/hostapd/Android.mk b/hostapd/Android.mk
>> > @@ -24,6 +24,9 @@ L_CFLAGS += -DVERSION_STR_POSTFIX=\"-$(PLATFORM_VERSION)\"
>> > +# Disable unused parameter warnings
>> > +L_CFLAGS += -Wno-unused-parameter
>>
>> Wouldn't it be better to fix the actual warnings?
>
> Depends on what you mean by fixing them.. I did not even know that
> someone would compile with -Wunused-parameter (or well, maybe more
> likely with -Wextra without disabling this specific warning). This
> results in 3522 "unused parameter" warning with my build configuration.

Apparently is what Android build system is doing - it sets -Wextra, so
-Wunused-parameter will be very helpful to avoid these 3522 warnings.

>
> I'm using "-Wextra -Wno-unused-parameter" for a good reason of getting
> rid of these bogus warnings. It is not an error, or worth a warning,
> IMHO, if a function does not use every single parameter. There is a
> large number of function pointers in wpa_supplicant/hostapd for
> implementing generic interfaces like EAP method operations or
> configuration parsers. Some EAP methods may need all the parameters,
> many don't. Trying to somehow encode every such location with
> __attribute__((unused)) to avoid this warning sounds both useless work
> and ugly in the end.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
Jouni Malinen April 24, 2014, 9:47 a.m. UTC | #5
Thanks, applied.
diff mbox

Patch

diff --git a/hostapd/Android.mk b/hostapd/Android.mk
index 6db82f8..74c9b27 100644
--- a/hostapd/Android.mk
+++ b/hostapd/Android.mk
@@ -24,6 +24,9 @@  L_CFLAGS += -DVERSION_STR_POSTFIX=\"-$(PLATFORM_VERSION)\"
 # Set Android log name
 L_CFLAGS += -DANDROID_LOG_NAME=\"hostapd\"
 
+# Disable unused parameter warnings
+L_CFLAGS += -Wno-unused-parameter
+
 ifeq ($(BOARD_WLAN_DEVICE), bcmdhd)
 L_CFLAGS += -DANDROID_P2P
 endif
diff --git a/wpa_supplicant/Android.mk b/wpa_supplicant/Android.mk
index c8fe1c2..f56267c 100644
--- a/wpa_supplicant/Android.mk
+++ b/wpa_supplicant/Android.mk
@@ -24,6 +24,9 @@  L_CFLAGS += -DVERSION_STR_POSTFIX=\"-$(PLATFORM_VERSION)\"
 # Set Android log name
 L_CFLAGS += -DANDROID_LOG_NAME=\"wpa_supplicant\"
 
+# Disable unused parameter warnings
+L_CFLAGS += -Wno-unused-parameter
+
 # Disable roaming in wpa_supplicant
 ifdef CONFIG_NO_ROAMING
 L_CFLAGS += -DCONFIG_NO_ROAMING