4:26 a.m.
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);
./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);
./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));
At first, seeing so many, I thought there must be a wrapper
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 ?
Jim
5:43 a.m.
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
6:17 a.m.
On Thu, Mar 30, 2006 at 06:43:10AM -0500, Daniel Veillard wrote:
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.
Well since it was trivial to catch those and exit I just spent the 2mn
needed to add this, this is fixed in CVS. For XML-RPC I expect the code
to be largely revamped before being integrated so I let it as is for the
moment.
Thanks,
Daniel
--
Daniel Veillard | Red Hat http://redhat.com/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
8:40 a.m.
On Čt, bře 30, 2006 at 07:17:58 -0500, Daniel Veillard wrote:
On Thu, Mar 30, 2006 at 06:43:10AM -0500, Daniel Veillard wrote:
> 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.
Well since it was trivial to catch those and exit I just spent the 2mn
needed to add this, this is fixed in CVS.
Thanks!
Karel
--
Karel Zak <kzak(a)redhat.com>
6625
days inactive
6625
days old
3 comments
3 participants
participants (3)
-
Daniel Veillard -
Jim Meyering -
Karel Zak