[libvirt] [PATCH] util: Fix virStorageBackendIQNFound() to work on FreeBSD

Despite being standardized in POSIX.1-2008, the 'm' sscanf() modifier is currently not available on FreeBSD. Reimplement parsing without sscanf() to work around the issue. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/viriscsi.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index e2c80acaaa..96934df948 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -146,8 +146,10 @@ virStorageBackendIQNFound(const char *initiatoriqn, line = outbuf; while (line && *line) { + char *current = line; char *newline; - int num; + char *next; + int i; if (!(newline = strchr(line, '\n'))) break; @@ -156,15 +158,29 @@ virStorageBackendIQNFound(const char *initiatoriqn, VIR_FREE(iface); VIR_FREE(iqn); - num = sscanf(line, "%ms %*[^,],%*[^,],%*[^,],%*[^,],%ms", &iface, &iqn); - if (num != 2) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("malformed output of %s: %s"), - ISCSIADM, line); + /* Find the first space, copy everything up to that point into + * iface and move past it to continue processing */ + if (!(next = strchr(current, ' '))) + goto error; + + if (VIR_STRNDUP(iface, current, (next - current)) < 0) goto cleanup; + + current = next + 1; + + /* There are five comma separated fields after iface and we only + * care about the last one, so we need to skip four commas and + * copy whatever's left into iqn */ + for (i = 0; i < 4; i++) { + if (!(next = strchr(current, ','))) + goto error; + current = next + 1; } + if (VIR_STRDUP(iqn, current) < 0) + goto cleanup; + if (STREQ(iqn, initiatoriqn)) { VIR_STEAL_PTR(*ifacename, iface); @@ -186,6 +202,12 @@ virStorageBackendIQNFound(const char *initiatoriqn, VIR_FREE(outbuf); virCommandFree(cmd); return ret; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("malformed output of %s: %s"), + ISCSIADM, line); + goto cleanup; } -- 2.17.1

On 07/25/2018 11:30 AM, Andrea Bolognani wrote:
Despite being standardized in POSIX.1-2008, the 'm' sscanf() modifier is currently not available on FreeBSD.
Reimplement parsing without sscanf() to work around the issue.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/viriscsi.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-)
Le sigh. So apparently we can't take 10 years old standard for granted and have to use this ugly (as in not obvious) style of parsing. One possibility is to go to BSD guys and ask them to comply with POSIX, but that is very likely to fail. So as much as I hate to write this, ACK Michal

On 07/25/2018 07:17 AM, Michal Privoznik wrote:
On 07/25/2018 11:30 AM, Andrea Bolognani wrote:
Despite being standardized in POSIX.1-2008, the 'm' sscanf() modifier is currently not available on FreeBSD.
Reimplement parsing without sscanf() to work around the issue.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/viriscsi.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-)
Le sigh. So apparently we can't take 10 years old standard for granted and have to use this ugly (as in not obvious) style of parsing. One possibility is to go to BSD guys and ask them to comply with POSIX, but that is very likely to fail. So as much as I hate to write this,
ACK
Michal
Should have gone with the overly intrusive virStringSplit[Count] option like I suggested during review ;-) [which is at least partially open coded here, but I digress]. John

On Wed, 2018-07-25 at 07:57 -0400, John Ferlan wrote:
Should have gone with the overly intrusive virStringSplit[Count] option like I suggested during review ;-) [which is at least partially open coded here, but I digress].
I didn't follow the review, I just jumped on it when I spotted the build failures on CentOS CI... If you can come up with a better version that still works on FreeBSD I'm pretty sure nobody will mind :) -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
John Ferlan
-
Michal Privoznik