On Thu, Apr 24, 2008 at 10:42:34PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
+ 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 defined a new convenience function STRPREFIX(str, prefix) which applies strlen(prefix) so we avoid the hardcoding. I didn't use sizeof(prefix) because someone might pass a non-literal string as the prefix and thta would endup as sizeof(char*) rather than the real length.
+ } 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.
Fix this to chcek offset for NULL.
+ tmp = sexpr_node(root, "domain/image/hvm/parallel"); + if (tmp && STRNEQ(tmp, "none")) { + /* XXX does XenD stuff parallel port tty info into xenstore somewhere ? */ + if (xend_parse_sexp_desc_char(conn, &buf, "parallel", 0, tmp, NULL) < 0) + goto error; + } + } else { + /* Paravirt always has a console */ + if (tty) { + virBufferVSprintf(&buf, " <console type='pty' tty='%s'>\n", tty); + virBufferVSprintf(&buf, " <source path='%s'/>\n", tty); + } else { + virBufferAddLit(&buf, " <console type='pty'>\n"); + } + virBufferAddLit(&buf, " <target port='0'/>\n"); + virBufferAddLit(&buf, " </console>\n"); } + free(tty);
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.
As discussed, posted new API to make virBuffer* safer. Regards, Daniel -- |: 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 :|