| Message ID | 4EDD8EBC.5040205@ozlabs.org |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans <matt@ozlabs.org> wrote: > term_getc()'s int c has one byte written into it (at its lowest address) by > read_in_full(). This is expected to be the least significant byte, but that > isn't the case on BE! Use correct type, unsigned char. A similar issue exists > in term_getc_iov(), which needs to write a char to the iov rather than an int. > > Signed-off-by: Matt Evans <matt@ozlabs.org> > --- > tools/kvm/term.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/tools/kvm/term.c b/tools/kvm/term.c > index fb5d71c..440884e 100644 > --- a/tools/kvm/term.c > +++ b/tools/kvm/term.c > @@ -30,11 +30,10 @@ int term_fds[4][2]; > > int term_getc(int who, int term) > { > - int c; > + unsigned char c; > > if (who != active_console) > return -1; > - > if (read_in_full(term_fds[term][TERM_FD_IN], &c, 1) < 0) > return -1; > > @@ -84,7 +83,7 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term) > if (c < 0) > return 0; > > - *((int *)iov[TERM_FD_IN].iov_base) = c; > + *((char *)iov[TERM_FD_IN].iov_base) = (char)c; > > return sizeof(char); > } Looks OK to me. Asias? -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/06/2011 06:24 PM, Pekka Enberg wrote: > On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans <matt@ozlabs.org> wrote: >> term_getc()'s int c has one byte written into it (at its lowest address) by >> read_in_full(). This is expected to be the least significant byte, but that >> isn't the case on BE! Use correct type, unsigned char. A similar issue exists >> in term_getc_iov(), which needs to write a char to the iov rather than an int. >> >> Signed-off-by: Matt Evans <matt@ozlabs.org> >> --- >> tools/kvm/term.c | 5 ++--- >> 1 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/tools/kvm/term.c b/tools/kvm/term.c >> index fb5d71c..440884e 100644 >> --- a/tools/kvm/term.c >> +++ b/tools/kvm/term.c >> @@ -30,11 +30,10 @@ int term_fds[4][2]; >> >> int term_getc(int who, int term) >> { >> - int c; >> + unsigned char c; >> >> if (who != active_console) >> return -1; >> - We can drop this line too. c &= 0xff; >> if (read_in_full(term_fds[term][TERM_FD_IN], &c, 1) < 0) >> return -1; >> >> @@ -84,7 +83,7 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term) >> if (c < 0) >> return 0; >> >> - *((int *)iov[TERM_FD_IN].iov_base) = c; >> + *((char *)iov[TERM_FD_IN].iov_base) = (char)c; >> >> return sizeof(char); >> } > > Looks OK to me. Asias? > Otherwise, looks fine to me.
Hi Asias, On 06/12/11 23:00, Asias He wrote: > On 12/06/2011 06:24 PM, Pekka Enberg wrote: >> On Tue, Dec 6, 2011 at 5:40 AM, Matt Evans <matt@ozlabs.org> wrote: >>> term_getc()'s int c has one byte written into it (at its lowest address) by >>> read_in_full(). This is expected to be the least significant byte, but that >>> isn't the case on BE! Use correct type, unsigned char. A similar issue exists >>> in term_getc_iov(), which needs to write a char to the iov rather than an int. >>> >>> Signed-off-by: Matt Evans <matt@ozlabs.org> >>> --- >>> tools/kvm/term.c | 5 ++--- >>> 1 files changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/tools/kvm/term.c b/tools/kvm/term.c >>> index fb5d71c..440884e 100644 >>> --- a/tools/kvm/term.c >>> +++ b/tools/kvm/term.c >>> @@ -30,11 +30,10 @@ int term_fds[4][2]; >>> >>> int term_getc(int who, int term) >>> { >>> - int c; >>> + unsigned char c; >>> >>> if (who != active_console) >>> return -1; >>> - > > We can drop this line too. > > c &= 0xff; D'oh! Of course it can -- I'll remove it & repost. >>> if (read_in_full(term_fds[term][TERM_FD_IN], &c, 1) < 0) >>> return -1; >>> >>> @@ -84,7 +83,7 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term) >>> if (c < 0) >>> return 0; >>> >>> - *((int *)iov[TERM_FD_IN].iov_base) = c; >>> + *((char *)iov[TERM_FD_IN].iov_base) = (char)c; >>> >>> return sizeof(char); >>> } >> >> Looks OK to me. Asias? >> > > Otherwise, looks fine to me. Thanks for reviewing! Matt -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tools/kvm/term.c b/tools/kvm/term.c index fb5d71c..440884e 100644 --- a/tools/kvm/term.c +++ b/tools/kvm/term.c @@ -30,11 +30,10 @@ int term_fds[4][2]; int term_getc(int who, int term) { - int c; + unsigned char c; if (who != active_console) return -1; - if (read_in_full(term_fds[term][TERM_FD_IN], &c, 1) < 0) return -1; @@ -84,7 +83,7 @@ int term_getc_iov(int who, struct iovec *iov, int iovcnt, int term) if (c < 0) return 0; - *((int *)iov[TERM_FD_IN].iov_base) = c; + *((char *)iov[TERM_FD_IN].iov_base) = (char)c; return sizeof(char); }
term_getc()'s int c has one byte written into it (at its lowest address) by read_in_full(). This is expected to be the least significant byte, but that isn't the case on BE! Use correct type, unsigned char. A similar issue exists in term_getc_iov(), which needs to write a char to the iov rather than an int. Signed-off-by: Matt Evans <matt@ozlabs.org> --- tools/kvm/term.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html