On Thu, Mar 30, 2006 at 12:26:31PM +0200, Jim Meyering wrote:
> Hello,
>
> In browsing through the code, I noticed that there are many
> places in which the code doesn't check for malloc returning NULL.
> E.g.,
>
> ./virsh.c: ids = malloc(sizeof(int) * maxid);
> ./virsh.c- virConnectListDomains(ctl->conn, &ids[0], maxid);
Actually virConnectListDomains which is in the library will check
the pointer. It won't crash in the library !
> ./virsh.c: *res = malloc(sz + 1);
> ./virsh.c- memcpy(*res, tkstr, sz);
>
> ./virsh.c: res = malloc(strlen(name) + 3);
> ./virsh.c- sprintf(res, "--%s", name);
virsh.c is code for the command line tool, I would say it's less critical
there than in the library if malloc fails and it leads to a crash.
But I'm sure Karel and me will enjoy a patch to fix this.
> ./xmlrpc.c: buf = malloc(sizeof(*buf));
> ./xmlrpc.c- buf->size = 1024;
>
> ./xmlrpc.c: xmlRpcValueDictElementPtr ret = malloc(sizeof(*ret));
> ./xmlrpc.c- xmlNodePtr cur;
> ./xmlrpc.c-
> ./xmlrpc.c- memset(ret, 0, sizeof(*ret));
I plead non-guilty for this code, moreover the code is not used yet,
it will be fixed before I make use of it.
> At first, seeing so many,
false alerts ...
> that handles the case of a NULL return value. But there are
> many places where the code *does* check for NULL.
>
> This may be a clue. Maybe it's planned...
>
> ./libvirt.c: * TODO:
> ...
> ./libvirt.c: * - memory wrappers for malloc/free ?
unrelated. And a wrapper could not fix a missing test anyway (exit()
is part of my NEVER_DO_THAT_IN_A_LIBRARY).
What it relates to is something like xmlMemSetup in libxml2
http://xmlsoft.org/html/libxml-xmlmemory.html#xmlMemSetup
allowing to trap all memory allocations and deallocations from the library,
but it's mostly used in embedded systems and I'm not sure libvirt really need
this. At least it doesn't sound urgent.
Daniel
--
Daniel Veillard | Red Hat http://redhat.com/
veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/