Patchwork [U-Boot,v5,3/5] net: fix potential compiler warning

login
register
mail settings
Submitter Michael Walle
Date May 11, 2012, 10:50 p.m.
Message ID <1336776649-9316-4-git-send-email-michael@walle.cc>
Download mbox | patch
Permalink /patch/158625/
State Superseded
Delegated to: Prafulla Wadaskar
Headers show

Comments

Michael Walle - May 11, 2012, 10:50 p.m.
Future compiler versions may generate a "too many arguments for functions"
warning.

Signed-off-by: Michael Walle <michael@walle.cc>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
---
 net/eth.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)
Joe Hershberger - May 16, 2012, 1:11 a.m.
Hi Michael,

On Fri, May 11, 2012 at 5:50 PM, Michael Walle <michael@walle.cc> wrote:
> Future compiler versions may generate a "too many arguments for functions"
> warning.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> ---
>  net/eth.c |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/net/eth.c b/net/eth.c
> index c9f05d8..afce863 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -58,7 +58,12 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
>                                 uchar *enetaddr)
>  {
>        char enetvar[32];
> -       sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> +
> +       if (index)
> +               sprintf(enetvar, "%s%daddr", base_name, index);
> +       else
> +               sprintf(enetvar, "%saddr", base_name);
> +
>        return eth_getenv_enetaddr(enetvar, enetaddr);
>  }
>
> @@ -66,7 +71,12 @@ static int eth_mac_skip(int index)
>  {
>        char enetvar[15];
>        char *skip_state;
> -       sprintf(enetvar, index ? "eth%dmacskip" : "ethmacskip", index);
> +
> +       if (index)
> +               sprintf(enetvar, "eth%dmacskip", index);
> +       else
> +               sprintf(enetvar, "ethmacskip");
> +
>        return ((skip_state = getenv(enetvar)) != NULL);
>  }

This seems like this could improve by consolidating the logic to come
up with the ethaddr variable name in an inline function.  Especially
since you are about to add another use in the setenv case.  Do you
agree, Mike?  Or is 3 uses of a relatively simple algorithm better
inlined than separated?

-Joe

Patch

diff --git a/net/eth.c b/net/eth.c
index c9f05d8..afce863 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -58,7 +58,12 @@  int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 				 uchar *enetaddr)
 {
 	char enetvar[32];
-	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+
+	if (index)
+		sprintf(enetvar, "%s%daddr", base_name, index);
+	else
+		sprintf(enetvar, "%saddr", base_name);
+
 	return eth_getenv_enetaddr(enetvar, enetaddr);
 }
 
@@ -66,7 +71,12 @@  static int eth_mac_skip(int index)
 {
 	char enetvar[15];
 	char *skip_state;
-	sprintf(enetvar, index ? "eth%dmacskip" : "ethmacskip", index);
+
+	if (index)
+		sprintf(enetvar, "eth%dmacskip", index);
+	else
+		sprintf(enetvar, "ethmacskip");
+
 	return ((skip_state = getenv(enetvar)) != NULL);
 }