Re: [libvirt] [PATCH 3/3] virISCSIGetSession: Don't leak memory

On 04/05/2017 04:16 PM, Martin Kletzander wrote: > On Wed, Apr 05, 2017 at 03:11:52PM -0400, John Ferlan wrote: >> >> >> On 04/05/2017 04:50 AM, Michal Privoznik wrote: >>> This function runs an iscsi command and parses its output. >>> However, due to the nature of things, virISCSIExtractSession() >>> callback can be called multiple times. In each run it would >>> allocate new memory and overwrite the variable where we keep >>> pointer to it and thus leaking old allocations. >>> >>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>> --- >>> src/util/viriscsi.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c >>> index 504ffbd..9c6fde0 100644 >>> --- a/src/util/viriscsi.c >>> +++ b/src/util/viriscsi.c >>> @@ -52,7 +52,8 @@ virISCSIExtractSession(char **const groups, >>> { >>> struct virISCSISessionData *data = opaque; >>> >>> - if (STREQ(groups[1], data->devpath)) >>> + if (!data->devpath && >>> + STREQ(groups[1], data->devpath)) >>> return VIR_STRDUP(data->session, groups[0]); >>> return 0; >>> } >>> >> >> I see you fixed your typo "!data->devpath" to "!data->session" before > > I wonder how I missed that, maybe because this wasn't the first time I > saw the patch O:-) > >> pushing, but the reality is this is a no-op considering at most we can >> only match once, but because virCommandRunRegex only bails if the return > > It is not no-op. The function is ran on every line of output and, as > we've noticed on Michal's machine, he got 2 lines with the same > group[1], so to speak. > So what's the output from a "iscsiadm --mode session" on Michal's machine? And then if there is a duplicate - how did it happen?