[PATCH] ethtool: return non-zero from do_sset() when any of ETHTOOL_{G,S}* fails

[PATCH] ethtool: return non-zero from do_sset() when any of ETHTOOL_{G,S}* fails

[Ivan Vecera]

Function do_sset returns unconditionally zero error code without regard
to return value of ETHTOOL_{G,S}* ioctls.

Cc: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 ethtool.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index bf583f3..c004d41 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2357,7 +2357,7 @@ static int do_sset(struct cmd_context *ctx)
 	int argc = ctx->argc;
 	char **argp = ctx->argp;
 	int i;
-	int err;
+	int err, result = 0;
 
 	for (i = 0; i < ARRAY_SIZE(flags_msglvl); i++)
 		flag_to_cmdline_info(flags_msglvl[i].name,
@@ -2617,6 +2617,7 @@ static int do_sset(struct cmd_context *ctx)
 				fprintf(stderr, "  not setting transceiver\n");
 			if (mdix_wanted != -1)
 				fprintf(stderr, "  not setting mdix\n");
+			result = err;
 		}
 	}
 
@@ -2650,6 +2651,7 @@ static int do_sset(struct cmd_context *ctx)
 				fprintf(stderr, "  not setting wol\n");
 			if (sopass_change)
 				fprintf(stderr, "  not setting sopass\n");
+			result = err;
 		}
 	}
 
@@ -2668,9 +2670,10 @@ static int do_sset(struct cmd_context *ctx)
 			if (err < 0)
 				perror("Cannot set new msglvl");
 		}
+		result = err;
 	}
 
-	return 0;
+	return result;
 }
 
 static int do_gregs(struct cmd_context *ctx)
-- 
2.0.5

Re: [PATCH] ethtool: return non-zero from do_sset() when any of ETHTOOL_{G,S}* fails

[Ivan Vecera]

On 02/06/2015 04:24 PM, Ivan Vecera wrote:
> Function do_sset returns unconditionally zero error code without regard
> to return value of ETHTOOL_{G,S}* ioctls.
>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
>   ethtool.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/ethtool.c b/ethtool.c
> index bf583f3..c004d41 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -2357,7 +2357,7 @@ static int do_sset(struct cmd_context *ctx)
>   	int argc = ctx->argc;
>   	char **argp = ctx->argp;
>   	int i;
> -	int err;
> +	int err, result = 0;
>
>   	for (i = 0; i < ARRAY_SIZE(flags_msglvl); i++)
>   		flag_to_cmdline_info(flags_msglvl[i].name,
> @@ -2617,6 +2617,7 @@ static int do_sset(struct cmd_context *ctx)
>   				fprintf(stderr, "  not setting transceiver\n");
>   			if (mdix_wanted != -1)
>   				fprintf(stderr, "  not setting mdix\n");
> +			result = err;
>   		}
>   	}
>
> @@ -2650,6 +2651,7 @@ static int do_sset(struct cmd_context *ctx)
>   				fprintf(stderr, "  not setting wol\n");
>   			if (sopass_change)
>   				fprintf(stderr, "  not setting sopass\n");
> +			result = err;
>   		}
>   	}
>
> @@ -2668,9 +2670,10 @@ static int do_sset(struct cmd_context *ctx)
>   			if (err < 0)
>   				perror("Cannot set new msglvl");
>   		}
> +		result = err;
>   	}
>
> -	return 0;
> +	return result;
>   }
>
>   static int do_gregs(struct cmd_context *ctx)
>
Ben, any comment on this?

Ivan

Re: [PATCH] ethtool: return non-zero from do_sset() when any of ETHTOOL_{G,S}* fails

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 567 bytes --]

On Fri, 2015-02-06 at 16:24 +0100, Ivan Vecera wrote:
> Function do_sset returns unconditionally zero error code without regard
> to return value of ETHTOOL_{G,S}* ioctls.
[...]

This behaviour is stupid, but it's what ethtool has always done and what
some scripts will depend on now.  If you can think of a simple way for
scripts to opt-in to sensible exit codes, please submit that.

Also, negative exit codes aren't sensible; please just use 0 and 1.

Ben.

-- 
Ben Hutchings
Quantity is no substitute for quality, but it's the only one we've got.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]