Date: Thu, 11 Aug 2016 21:53:56 +0200
From: Natanael Copa <ncopa@alpinelinux.org>
To: Jann - Ove Risvik <jann.ove@usaklig.com>
Cc: alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports] [PATCH] main/open-iscsi: musl fix for error
 handling. fixes bug #4802
Message-ID: <20160811215356.31e47f57@ncopa-desktop.copa.dup.pw>
In-Reply-To: <20160708015558.1578-1-jann.ove@usaklig.com>
References: <20160708015558.1578-1-jann.ove@usaklig.com>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Fri,  8 Jul 2016 03:55:58 +0200
Jann - Ove Risvik <jann.ove@usaklig.com> wrote:

> Fixes bug #4802: http://bugs.alpinelinux.org/issues/4802
> 
> Seems like iscsiadm relies on checking optopt for checking if there's an error, instead of checking whether opterr is set or if getopt returns '?' and then using optopt to retrieve the option that it didn't recognize.
> 
> For some reason musl sets optopt to the last option parsed even if it was valid. Is musl supposed to do that? glibc doesn't behave this way. The posix standard for getopt also reads like optopt is only supposed to be set when there's an error.
> 
> (Only when I was looking for what to put in the email subject line, after I figured it all out did I notice that Brian Angus already submitted a fix earlier. Figured I'd sent this one anyway; that fix removes error output when presented with actual unrecognised arguments.)


Thank you very much for your your analyze! Good job!

I mentioned it to the musl devs and it might make sense to fix this in
musl in this case. I think I'll still add a workaround to open-iscsi.

> ---
>  main/open-iscsi/APKBUILD         |  8 ++++----
>  main/open-iscsi/musl-fixes.patch | 24 ++++++++++++++++++++++++
>  2 files changed, 28 insertions(+), 4 deletions(-)

...

> diff --git a/main/open-iscsi/musl-fixes.patch b/main/open-iscsi/musl-fixes.patch
> index 35f11e1..910b485 100644
> --- a/main/open-iscsi/musl-fixes.patch
> +++ b/main/open-iscsi/musl-fixes.patch
> @@ -29,3 +29,27 @@
>   #include <dirent.h>
>   #include <limits.h>
>   #include <sys/stat.h>
> +--- ./usr/iscsiadm.c.orig
> ++++ ./usr/iscsiadm.c
> +@@ -2553,7 +2553,10 @@ main(int argc, char **argv)
> + 			return 0;
> + 		case 'h':
> + 			usage(0);
> +-		}
> ++
> ++		case '?':
> ++			log_error("unrecognized character '%c'", optopt);
> ++		}	
> + 
> + 		if (name && value) {
> + 			param = idbm_alloc_user_param(name, value);
> +@@ -2568,8 +2571,7 @@ main(int argc, char **argv)
> + 		}
> + 	}
> + 
> +-	if (optopt) {
> +-		log_error("unrecognized character '%c'", optopt);
> ++	if (opterr) {
> + 		rc = ISCSI_ERR_INVAL;
> + 		goto free_ifaces;
> + 	}


As you noticed you cannot use opterr this way.

-nc


---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---