[RFA] Fix leaks in all the linux osdata annex transfers + code factorization.

Simon Marchi simon.marchi@polymtl.ca
Tue Dec 11 04:55:00 GMT 2018


On 2018-12-08 13:07, Philippe Waroquiers wrote:
> Valgrind reports leaks in all linux osdata annex transfers of 
> linux-osdata.c.
> 
> A typical leak (this one is of gdb.base/info-os) is:
> ==10592== VALGRIND_GDB_ERROR_BEGIN
> ==10592== 65,536 bytes in 1 blocks are definitely lost in loss record
> 3,175 of 3,208
> ==10592==    at 0x4C2E273: realloc (vg_replace_malloc.c:826)
> ==10592==    by 0x409B0C: xrealloc (common-utils.c:62)
> ==10592==    by 0x408BC3: buffer_grow(buffer*, char const*, unsigned
> long) [clone .part.1] (buffer.c:40)
> ==10592==    by 0x5263DF: linux_xfer_osdata_processes(unsigned char*,
> unsigned long, unsigned long) (linux-osdata.c:370)
> ==10592==    by 0x520875: linux_nat_xfer_osdata (linux-nat.c:4214)
> ...
> 
> The leaks are created because the linux_xfer_osdata_* functions
> transfer the ownership of their 'static struct buffer' memory
> to their 'static char *buf' local var, but then call buffer_free
> instead of xfree-ing buf.
> 
> I see no reason why the ownership of the memory has to be transferred
> from a local var to another local var, so the fix consists in dropping
> the 'static char *buf' and accessing the struct buffer memory where 
> needed.
> 
> Also, because this bug was replicated in all functions, and there was
> a non neglectible amount of duplicated code, the setup and usage
> of the 'static struct buffer' is factorized in a new function
> common_getter.  The buffer for a specific annex is now a member
> of the struct osdata_type instead of being a static var of each
> linux_xfer_osdata_* function.
> 
> Thanks to this, all the linux_xfer_osdata_* do not have
> anymore any logic related to the partial transfer of data: they now
> only build the xml data in a struct buffer.
> This all removes about 300 SLOC.
> 
> Note: git diff/git format-patch shows a lot of differences only due
> to space changes/indentation changes.
> So, git diff -w helps to look only at the relevant differences.
> 
> gdb/ChangeLog
> 2018-12-08  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* nat/linux-osdata.c : Removed various trailing spaces.
> 	(common_getter): New function.
> 	(struct osdata_type): Change getter to take_snapshot.
> 	Add LONGEST len_avail and struct buffer buffer.
> 	Change all elements in the initializer.
> 	Add an element for the list of types.
> 	(linux_xfer_osdata_info_os_types): New function.
> 	(linux_common_xfer_osdata): Use common_getter for the list of types.
> 	Replace getter call by common_getter.
> 	(linux_xfer_osdata_cpus): Remove args READBUF, OFFSET, LEN.
> 	Add arg BUFFER.  Only keep the code that adds data in BUFFER.
> 	(linux_xfer_osdata_fds): Likewise.
> 	(linux_xfer_osdata_modules): Likewise.
> 	(linux_xfer_osdata_msg): Likewise.
> 	(linux_xfer_osdata_processes): Likewise.
> 	(linux_xfer_osdata_processgroups): Likewise.
> 	(linux_xfer_osdata_sem): Likewise.
> 	(linux_xfer_osdata_shm): Likewise.
> 	(linux_xfer_osdata_isockets): Likewise.
> 	(linux_xfer_osdata_threads): Likewise.

Thanks, this is a nice cleanup, this LGTM.

Would it be possible to split the changes that remove the trailing 
spaces in a separate (obvious) patch?

Simon



More information about the Gdb-patches mailing list