| Message ID | 1295296566-30287-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | Accepted |
| Headers | show |
Stefan Weil <weil@mail.berlios.de> writes: > Extract from "man realloc": > "If realloc() fails the original block is left untouched; > it is not freed or moved." > > Fix a possible memory leak (reported by cppcheck). > > Cc: Riku Voipio <riku.voipio@iki.fi> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> Sidestep the problem via qemu_realloc() instead?
Am 18.01.2011 09:26, schrieb Markus Armbruster: > Stefan Weil <weil@mail.berlios.de> writes: > >> Extract from "man realloc": >> "If realloc() fails the original block is left untouched; >> it is not freed or moved." >> >> Fix a possible memory leak (reported by cppcheck). >> >> Cc: Riku Voipio <riku.voipio@iki.fi> >> Signed-off-by: Stefan Weil <weil@mail.berlios.de> > > Sidestep the problem via qemu_realloc() instead? The same change was applied to bsd-user/elfload.c. As symbol loading is not essential in most applications, returning after out-of-memory should be better than aborting (that's what qemu_realloc does).
Stefan Weil <weil@mail.berlios.de> writes: > Am 18.01.2011 09:26, schrieb Markus Armbruster: >> Stefan Weil <weil@mail.berlios.de> writes: >> >>> Extract from "man realloc": >>> "If realloc() fails the original block is left untouched; >>> it is not freed or moved." >>> >>> Fix a possible memory leak (reported by cppcheck). >>> >>> Cc: Riku Voipio <riku.voipio@iki.fi> >>> Signed-off-by: Stefan Weil <weil@mail.berlios.de> >> >> Sidestep the problem via qemu_realloc() instead? > > The same change was applied to bsd-user/elfload.c. > > As symbol loading is not essential in most applications, > returning after out-of-memory should be better than > aborting (that's what qemu_realloc does). Unless the requested size is *really* large, I'd expect this to stave off the out-of-memory failure for a few microseconds at best.
Am 18.01.2011 18:51, schrieb Markus Armbruster: > Stefan Weil<weil@mail.berlios.de> writes: > > >> Am 18.01.2011 09:26, schrieb Markus Armbruster: >> >>> Stefan Weil<weil@mail.berlios.de> writes: >>> >>> >>>> Extract from "man realloc": >>>> "If realloc() fails the original block is left untouched; >>>> it is not freed or moved." >>>> >>>> Fix a possible memory leak (reported by cppcheck). >>>> >>>> Cc: Riku Voipio<riku.voipio@iki.fi> >>>> Signed-off-by: Stefan Weil<weil@mail.berlios.de> >>>> >>> Sidestep the problem via qemu_realloc() instead? >>> >> The same change was applied to bsd-user/elfload.c. >> >> As symbol loading is not essential in most applications, >> returning after out-of-memory should be better than >> aborting (that's what qemu_realloc does). >> > Unless the requested size is *really* large, I'd expect this to stave > off the out-of-memory failure for a few microseconds at best. > If realloc fails, some memory is released before returning, so maybe you would be surprised that your program finishes without any more problems :-)
On 18 January 2011 17:51, Markus Armbruster <armbru@redhat.com> wrote: > Stefan Weil <weil@mail.berlios.de> writes: >> Am 18.01.2011 09:26, schrieb Markus Armbruster: >>> Stefan Weil <weil@mail.berlios.de> writes: >>>> Extract from "man realloc": >>>> "If realloc() fails the original block is left untouched; >>>> it is not freed or moved." >>> Sidestep the problem via qemu_realloc() instead? >> >> The same change was applied to bsd-user/elfload.c. >> >> As symbol loading is not essential in most applications, >> returning after out-of-memory should be better than >> aborting (that's what qemu_realloc does). > > Unless the requested size is *really* large, I'd expect this to stave > off the out-of-memory failure for a few microseconds at best. Yeah, but the patch is OK, it fixes an actual bug and it does so in line with the malloc-failure handling of the rest of the function. It doesn't seem to me to be important enough an issue to worry about one way or the other. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
diff --git a/linux-user/elfload.c b/linux-user/elfload.c index ab03e16..f9bd849 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1474,7 +1474,7 @@ static void load_symbols(struct elfhdr *hdr, int fd, abi_ulong load_bias) struct elf_shdr *shdr; char *strings; struct syminfo *s; - struct elf_sym *syms; + struct elf_sym *syms, *new_syms; shnum = hdr->e_shnum; i = shnum * sizeof(struct elf_shdr); @@ -1543,12 +1543,14 @@ static void load_symbols(struct elfhdr *hdr, int fd, abi_ulong load_bias) that we threw away. Whether or not this has any effect on the memory allocation depends on the malloc implementation and how many symbols we managed to discard. */ - syms = realloc(syms, nsyms * sizeof(*syms)); - if (syms == NULL) { + new_syms = realloc(syms, nsyms * sizeof(*syms)); + if (new_syms == NULL) { free(s); + free(syms); free(strings); return; } + syms = new_syms; qsort(syms, nsyms, sizeof(*syms), symcmp);
Extract from "man realloc": "If realloc() fails the original block is left untouched; it is not freed or moved." Fix a possible memory leak (reported by cppcheck). Cc: Riku Voipio <riku.voipio@iki.fi> Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- linux-user/elfload.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)