On Thu, Apr 24, 2008 at 10:42:34PM +0200, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> > On Fri, Apr 18, 2008 at 09:27:05PM +0100, Daniel P. Berrange wrote:
> >> This patch updates Xen HVM to allow use of serial ¶llel ports, though
> >> XenD limits you to single one of each even though QEMU supports many.
> >> It also updates the <console> tag to support new syntax extensions.
> >
> > This version fixes a few memory leaks in the previous code...
>
> Hi Dan,
>
> > Index: src/xend_internal.c
> > ===================================================================
> > RCS file: /data/cvs/libvirt/src/xend_internal.c,v
> > retrieving revision 1.180
> > diff -u -p -r1.180 xend_internal.c
> > --- src/xend_internal.c 10 Apr 2008 16:54:54 -0000 1.180
> > +++ src/xend_internal.c 24 Apr 2008 15:10:00 -0000
> > @@ -1376,6 +1376,243 @@ xend_parse_sexp_desc_os(virConnectPtr xe
> > return(0);
> > }
> >
> > +
> > +int
> > +xend_parse_sexp_desc_char(virConnectPtr conn,
> > + virBufferPtr buf,
> > + const char *devtype,
> > + int portNum,
> > + const char *value,
> > + const char *tty)
> > +{
> > + const char *type;
> > + int telnet = 0;
> > + char *bindPort = NULL;
> > + char *bindHost = NULL;
> > + char *connectPort = NULL;
> > + char *connectHost = NULL;
> > + char *path = NULL;
> > + int ret = -1;
> > +
> > + if (value[0] == '/') {
> > + type = "dev";
> > + } else if (STREQLEN(value, "null", 4)) {
> > + type = "null";
> > + value = NULL;
> > + } else if (STREQLEN(value, "vc", 2)) {
> > + type = "vc";
> > + value = NULL;
> > + } else if (STREQLEN(value, "pty", 3)) {
> > + type = "pty";
> > + value = NULL;
> > + } else if (STREQLEN(value, "stdio", 5)) {
> > + type = "stdio";
> > + value = NULL;
> > + } else if (STREQLEN(value, "file:", 5)) {
> > + type = "file";
> > + value += 5;
> > + } else if (STREQLEN(value, "pipe:", 5)) {
> > + type = "pipe";
> > + value += 5;
> > + } else if (STREQLEN(value, "tcp:", 4)) {
> > + type = "tcp";
> > + value += 4;
> > + } else if (STREQLEN(value, "telnet:", 4)) {
>
> That "4" should be 7.
>
> It'd be nice to avoid the riskily redundant length,
> by using a STR* macro that requires a literal string S
> as argument #2, and uses sizeof(S)-1 as the length argument.
I'll see about adding a suitable macro for that.
> > + } else if (STREQ(type, "unix")) {
> > + const char *offset = strchr(value, ',');
> > + int dolisten = 0;
> > + if (offset)
> > + path = strndup(value, (offset - value));
> > + else
> > + path = strdup(value);
> > + if (path == NULL)
> > + goto no_memory;
> > +
> > + if (strstr(offset, ",listen") != NULL)
>
> If the strchr call above finds no comma, then
> "offset" will be NULL and strstr call will segfault.
Yes, that needs a check for offset != NULL in the conditional
before strstr.
> > virBufferAddLit(&buf, " </devices>\n");
> > virBufferAddLit(&buf, "</domain>\n");
>
> All of the virBufferAddLit and virBufferVSprintf calls
> above can fail with ENOMEM. I see that there are *many* more
> virBufferAddLit and virBufferVSprintf calls that ignore their
> return values (over 300), so maybe I'm missing something.
Yes, the entire set of Xen drivers is basically fubar wrt to checking
virBufferXXX return values. I felt that needed addresing independantly
changing everything in one go, so didn't try to address that in this
patch.
> > +int
> > +virDomainParseXMLOSDescHVMChar(virConnectPtr conn,
> > + char *buf,
> > + size_t buflen,
> > + xmlNodePtr node)
> > +{
> > + xmlChar *type = NULL;
>
> Can you use "const char *" above, instead of "xmlChar *"?
> If so, that'd let you remove most of those risky and
> ugly (const char*) casts below.
That'd just mean I needed to cast the to const char *
and back to xmlChar * in different places. eg all the
xmlGetProp and xmlFree calls. Its pretty unpleasant
either way around, so I kept the style the rest of the
file uses.
Dan.
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list