3:01 p.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Apr 18, 2008 at 09:25:28PM +0100, Daniel P. Berrange wrote:
> This supports the serial/character devices in QEMU. It is complete
> except for fact that I don't extract the TTY path for type='pty'.
> This needs addressing before merging
This patch now includes the ability to extract the TTY path for
serial and parallel devices, so we finally have 'virsh console'
working for QEMU. THat said, plain QEMU/KVM has busted serial
console which translates \r\n into \n\n, but I've sent a patch
for that upstream
Nice!
...
Index: src/qemu_conf.c
===================================================================
...
+static int qemudGenerateXMLChar(virBufferPtr buf,
+ const struct qemud_vm_chr_def *dev,
+ const char *type)
+{
+ const char *const types[] = {
+ "null",
+ "vc",
+ "pty",
+ "dev",
+ "file",
+ "pipe",
+ "stdio",
+ "udp",
+ "tcp",
+ "unix"
+ };
+ /*verify(ARRAY_CARDINALITY(types) == QEMUD_CHR_SRC_TYPE_LAST);*/
A couple days ago I relaxed the copyright on the gnulib module to
LGPLv2+, so you can go ahead and uncomment that, #include "verify.h",
and add "verify" to the list in bootstrap.
Index: src/qemu_driver.c
===================================================================
...
-static int qemudExtractMonitorPath(const char *haystack, char *path,
int pathmax) {
+static int qemudExtractMonitorPath(const char *haystack,
+ size_t *offset,
+ char *path, int pathmax) {
Thanks for shortening long lines ;-)
static const char needle[] = "char device redirected
to";
char *tmp;
- if (!(tmp = strstr(haystack, needle)))
+ /* First look for our magic string */
+ if (!(tmp = strstr(haystack + *offset, needle)))
return -1;
+ /* Grab all the trailing data */
strncpy(path, tmp+sizeof(needle), pathmax-1);
That should be sizeof(needle)-1.
Otherwise, if someone nasty gave you input ending with
"char device redirected to", the strncpy above would start
reading just past the NUL at the end of "haystack".
path[pathmax-1] = '\0';
- while (*path) {
- /*
- * The monitor path ends at first whitespace char
- * so lets search for it & NULL terminate it there
- */
- if (isspace(*path)) {
- *path = '\0';
+ /*
+ * And look for first whitespace character and nul terminate
+ * to mark end of the pty path
+ */
+ tmp = path;
+ while (*tmp) {
+ if (isspace(*tmp)) {
Since "tmp" has type "char", this causes trouble in an environment
where "char" is a signed type. When *tmp is larger than 127, it gets
sign-extended, and isspace can misbehave on the large negative number
(isspace is not defined for such values). Instead, do it like this:
if (isspace(*(unsigned char *)tmp)) {
or better, using the to_uchar function (from coreutils):
if (isspace(to_uchar(tmp))) {
/* Convert a possibly-signed character to an unsigned character. This is
a bit safer than casting to unsigned char, since it catches some type
errors that the cast doesn't. */
static inline unsigned char to_uchar (char ch) { return ch; }
I just happened upon this one, but have audited the rest of the code
for similar problems. To get an idea of the size of this task,
I got the list of is* functions from the man page and did this:
(tolower and toupper have the same limitation)
re=$(man isspace|grep is.....,.is|sed 's/ -.*//'|tr -s ', \n' \| \
|sed 's/^|//;s/|$//')
git grep -E "\b($re|tolower|toupper)\b"
Here's the output:
ChangeLog: Unlike in qemud.c, here we allow trailing "isspace", and in
ChangeLog: * src/virsh.c: Remove use of _GNU_SOURCE / isblank.
src/nodeinfo.c: while (*buf && isspace(*buf))
src/nodeinfo.c: while (*buf && isspace(*buf))
src/nodeinfo.c: && (*p == '\0' || *p == '.' ||
isspace(*p)))
src/nodeinfo.c: while (*buf && isspace(*buf))
src/nodeinfo.c: && (*p == '\0' || isspace(*p))
src/qemu_driver.c: if (isspace(*path)) {
src/sexpr.c: while (*ptr && !isspace(*ptr) && *ptr !=
')' && *ptr != '
src/stats_linux.c: if (!isdigit(path[4]) || path[4] == '0' ||
src/stats_linux.c: if (p && (!isdigit(*p) || *p == '0' ||
src/stats_linux.c: if (!isdigit(path[3]) || path[3] == '0' ||
src/util.c:#define TOLOWER(Ch) (isupper (Ch) ? tolower (Ch) : (Ch))
src/util.c: while (*p == '0' && isxdigit (p[1]))
src/util.c: while (*q == '0' && isxdigit (q[1]))
src/util.c: if (!isxdigit(*str))
src/virsh.c: if (!isdigit (cpulist[i])) {
src/virsh.c: else if (!isdigit (cpulist[i])) {
src/virsh.c: && isalnum((unsigned char) *(p + 2))) {
So I've just posted a patch to fix those.