On Mon, Sep 30, 2019 at 11:22:03AM -0500, Eric Blake wrote: > On 9/28/19 4:38 PM, Richard W.M. Jones wrote: > >On Fri, Sep 27, 2019 at 11:48:47PM -0500, Eric Blake wrote: > >>Fixes the fact that clients could not request the maximum string > >>length except with NBD_OPT_EXPORT_LEN. Updates the testsuite to > >>match. > >> > >>Signed-off-by: Eric Blake <[email protected]> > >>--- > > >>@@ -255,7 +255,7 @@ negotiate_handshake_newstyle_options (struct connection > >>*conn) > >> uint64_t version; > >> uint32_t option; > >> uint32_t optlen; > >>- char data[MAX_OPTION_LENGTH+1]; > >>+ CLEANUP_FREE char *data = NULL; > > > >Even though you have the CLEANUP here ... > > Scope is too wide. I need to sink the declaration... > > > > >> struct nbd_export_name_option_reply handshake_finish; > >> const char *optname; > >> uint64_t exportsize; > >>@@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct > >>connection *conn) > >> nbdkit_error ("client option data too long (%" PRIu32 ")", optlen); > >> return -1; > >> } > >>+ data = malloc (optlen + 1); /* Allowing a trailing NUL helps some uses > >>*/ > >>+ if (data == NULL) { > >>+ nbdkit_error ("malloc: %m"); > >>+ return -1; > >>+ } > > ...inside the while loop, so that each iteration of the loop frees > and reallocates a data buffer. > > > > >... when I run this patch series under valgrind I get mainly errors > >originating at this malloc: > > > >==1251605== 58 bytes in 4 blocks are definitely lost in loss record 4 of 10 > >==1251605== at 0x896180B: malloc (vg_replace_malloc.c:309) > >==1251605== by 0x11909F: protocol_handshake_newstyle > >(protocol-handshake-news > >tyle.c:288) > >==1251605== by 0x118804: protocol_handshake (protocol-handshake.c:55) > >==1251605== by 0x112080: handle_single_connection (connections.c:165) > >==1251605== by 0x11B84D: start_thread (sockets.c:276) > >==1251605== by 0x8BB74E1: start_thread (pthread_create.c:479) > >==1251605== by 0x8CD3642: clone (clone.S:95) > > > >I didn't look at it closely but there does appear to be a memory leak > >in this patch. > > Yep, and you pointed it out very nicely. I'll fix before pushing.
Ah yes of course, I didn't spot that there was a loop :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
