On 07/02/2018 09:39 AM, Peter Krempa wrote:
On Fri, Jun 29, 2018 at 17:01:49 +0200, Michal Privoznik wrote:
Firstly, we can utilize virCommandSetOutputBuffer() API which will collect the command output for us. Secondly, sscanf()-ing through each line is easier to understand (and more robust) than jumping over a string with strchr().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/viriscsi.c | 85 +++++++++++++++++++++-------------------------------- 1 file changed, 34 insertions(+), 51 deletions(-)
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 2e55b3c10b..44788056fd 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c
[...]
@@ -117,71 +116,56 @@ static int virStorageBackendIQNFound(const char *initiatoriqn, char **ifacename) { - int ret = IQN_ERROR, fd = -1; - char ebuf[64]; - FILE *fp = NULL; - char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL; + int ret = IQN_ERROR; + char *outbuf = NULL; + char *line = NULL; + char *iface = NULL; + char *iqn = NULL; virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "iface", NULL);
*ifacename = NULL;
- if (VIR_ALLOC_N(line, LINE_SIZE) != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not allocate memory for output of '%s'"), - ISCSIADM); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; - }
- memset(line, 0, LINE_SIZE); + /* Example of data we are dealing with: + * default tcp,<empty>,<empty>,<empty>,<empty> + * iser iser,<empty>,<empty>,<empty>,<empty> + * libvirt-iface-253db048 tcp,<empty>,<empty>,<empty>,iqn.2017-03.com.user:client + */
- virCommandSetOutputFD(cmd, &fd); - if (virCommandRunAsync(cmd, NULL) < 0) - goto cleanup; + line = outbuf; + while (line) {
This is severely misleading and makes this loop technically infinite. Or just checks that output buffer was not null. Both operations should be made explicit. Or you meant *line or line[0]
Okay, how about: while (line && *line) { Michal