GNOME Bugzilla – Bug 167969
[PATCH] gnome-system log may crash at startup plus leaks memory
Last modified: 2005-03-02 15:35:02 UTC
Steps to reproduce: 1. Launch gnome-system-log 2. 3. Stack trace: % gdb gnome-system-log (gdb) r Starting program: /usr/X11R6/bin/gnome-system-log Program received signal SIGSEGV, Segmentation fault.
+ Trace 55917
Thread 1 (LWP 100137)
Other information: The crash is not 100% reproducible, but the problem is with gnome_vfs_read_entire_file(). It returns a string buffer without the terminating NUL byte. This causes funky things to happen. While looking at this, I noticed that buffer itself was never freed. My attached patch corrects the crash, and fixes the buffer leak. As a pleasant side-effect, gnome-system-log now reports correctly formatted log data.
This is gnome-utils 2.9.91.
Created attachment 37708 [details] [review] Fix crash and memory leak in gnome-system-log
Shouldn't the bug in gnome-vfs be filed against gnome-vfs ? If this really fixes a crash, you can commit now, until the good fix is actually in gnome-vfs. The g_free part I have no problem with, thanks ! If you could file the appropriate bug against gnome-vfs that would be really nice (and put this new bug as a dependency of the present bug would rock even more...) Once you committed your patch, don't close this present bug - we'll close it once it's fixed on the gnome-vfs part. Please correct me if I'm wrong.
Well, the memory leak is definitely a logview problem. I also thing gnome_vfs_read_entire_file() is operating as designed. No where in the API documentation does it say it will return a NUL-terminated string buffer. I think this question is best left up to Alex. As for committing this, I really think it should be done in logview now, as you say. However, I'm not a committer, so someone with write access to the repo will need to do this. In the meantime, I'll run this by Alex, and update the bug as necessary.
I got a reply from Alex: From: Alexander Larsson <alexl@redhat.com> > You're right that currently, as designed, read_entire_file doesn't nul > terminate the buffer. However, maybe we should change it to do so, since > g_file_get_contents() does this. I can see this being useful in many > cases. I've written up a patch for gnome-vfs, and sent it to him. Given that, the NUL hack to logview can probably wait.
Alex committed the fix to gnome_vfs_read_entire_file(), so all that's left to do for logview is commit the memory leak portion of the patch. Thanks!
Created attachment 37869 [details] [review] Fix memory leak in gnome-system-log
Sorry, it seems like I missed the deadline for code freeze... :(
Nobody's stopping you from submitting this for inclusion in 2.10.0. Just mail release-team@gnome.org and ask for permission...
Ok, this is in now. Thanks !