Re: [Libvir] PATCH 2/4: Xen driver support for serial/paralle

"Daniel P. Berrange" <berrange(a)redhat.com&gt; 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.

> + } 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.

> + 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.