This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]