[Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Blue Swirl]

Revision: 6023
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6023
Author:   blueswir1
Date:     2008-12-14 09:30:41 +0000 (Sun, 14 Dec 2008)

Log Message:
-----------
Use a hex value instead of possibly ambiguous 8 bit character

Modified Paths:
--------------
    trunk/block-vvfat.c

Modified: trunk/block-vvfat.c
===================================================================
--- trunk/block-vvfat.c	2008-12-14 09:22:41 UTC (rev 6022)
+++ trunk/block-vvfat.c	2008-12-14 09:30:41 UTC (rev 6023)
@@ -1249,7 +1249,7 @@
 	unsigned char* c=(unsigned char*)direntry;
 	int i;
 	for(i=1;i<11 && c[i] && c[i]!=0xff;i+=2)
-#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = '\xB0'; j++;}
+#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = 0xb0; j++;}
 	    ADD_CHAR(c[i]);
 	for(i=14;i<26 && c[i] && c[i]!=0xff;i+=2)
 	    ADD_CHAR(c[i]);

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Johannes Schindelin]

Hi,

On Sun, 14 Dec 2008, Blue Swirl wrote:

> Revision: 6023
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6023
> Author:   blueswir1
> Date:     2008-12-14 09:30:41 +0000 (Sun, 14 Dec 2008)
> 
> Log Message:
> -----------
> Use a hex value instead of possibly ambiguous 8 bit character

/me is curious: how could buffer[j] = '\xb0' be ambiguous when buffer is 
of type char *?  It's not as if C did UTF-8 conversion with chars.

Besides...

> @@ -1249,7 +1249,7 @@
>  	unsigned char* c=(unsigned char*)direntry;
>  	int i;
>  	for(i=1;i<11 && c[i] && c[i]!=0xff;i+=2)
> -#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = '\xB0'; j++;}
> +#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = 0xb0; j++;}

in the meantime I think it would be more readable as

#define ADD_CHAR(c) buffer[j++] = (c) < ' ' ? '\xb0' : 'c';

Note that

- this code is only ever reached when DEBUG is defined, and

- this code still assumes that your terminal is ISO-8859-1, which is 
  typically wrong these days (UTF-8 is the de-facto standard).

Ciao,
Dscho

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Blue Swirl]

On 12/14/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
>
>  On Sun, 14 Dec 2008, Blue Swirl wrote:
>
>  > Revision: 6023
>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6023
>  > Author:   blueswir1
>  > Date:     2008-12-14 09:30:41 +0000 (Sun, 14 Dec 2008)
>  >
>  > Log Message:
>  > -----------
>  > Use a hex value instead of possibly ambiguous 8 bit character
>
>
> /me is curious: how could buffer[j] = '\xb0' be ambiguous when buffer is
>  of type char *?  It's not as if C did UTF-8 conversion with chars.

The diff does not show it properly, there was a 8 bit character
between the apostrophes, not \xb0. One day some compiler might want to
parse the source text as UTF-8, then byte B0 and apostrophe after it
could decode to something different with mysterious side effects. 0xb0
will not ever cause these problems, '\xb0' could work too.

>  Besides...
>
>
>  > @@ -1249,7 +1249,7 @@
>  >       unsigned char* c=(unsigned char*)direntry;
>  >       int i;
>  >       for(i=1;i<11 && c[i] && c[i]!=0xff;i+=2)
>  > -#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = '\xB0'; j++;}
>  > +#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = 0xb0; j++;}
>
>
> in the meantime I think it would be more readable as
>
>  #define ADD_CHAR(c) buffer[j++] = (c) < ' ' ? '\xb0' : 'c';
>
>  Note that
>
>  - this code is only ever reached when DEBUG is defined, and
>
>  - this code still assumes that your terminal is ISO-8859-1, which is
>   typically wrong these days (UTF-8 is the de-facto standard).

Patches welcome :)

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Johannes Schindelin]

Hi,

On Sun, 14 Dec 2008, Blue Swirl wrote:

> On 12/14/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> >  On Sun, 14 Dec 2008, Blue Swirl wrote:
> >
> >  > Revision: 6023
> >  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6023
> >  > Author:   blueswir1
> >  > Date:     2008-12-14 09:30:41 +0000 (Sun, 14 Dec 2008)
> >  >
> >  > Log Message:
> >  > -----------
> >  > Use a hex value instead of possibly ambiguous 8 bit character
> >
> >
> > /me is curious: how could buffer[j] = '\xb0' be ambiguous when buffer is
> >  of type char *?  It's not as if C did UTF-8 conversion with chars.
> 
> The diff does not show it properly, there was a 8 bit character between 
> the apostrophes, not \xb0.

Right.

> One day some compiler might want to parse the source text as UTF-8, then 
> byte B0 and apostrophe after it could decode to something different with 
> mysterious side effects.

This will not be the case unless sizeof(char) will be anything else than 
1.  Which will be, uhm, never.

> Patches welcome :)

[PATCH] block-vvfat: fix warnings and simplify ADD_CHAR when compiling with DEBUG

ADD_CHAR() now substitutes non-ASCII characters with '.' to avoid the
assumption that the terminal is ISO-8859-1 (\xb0 is the degree character
in that encoding, but an invalid character in UTF-8, which is the de-facto
standard these days).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 block-vvfat.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block-vvfat.c b/block-vvfat.c
index 1ac4472..1104cea 100644
--- a/block-vvfat.c
+++ b/block-vvfat.c
@@ -1242,14 +1242,14 @@ static void print_direntry(const direntry_t* direntry)
     int j = 0;
     char buffer[1024];
 
-    fprintf(stderr, "direntry 0x%x: ", (int)direntry);
+    fprintf(stderr, "direntry %p: ", direntry);
     if(!direntry)
 	return;
     if(is_long_name(direntry)) {
 	unsigned char* c=(unsigned char*)direntry;
 	int i;
 	for(i=1;i<11 && c[i] && c[i]!=0xff;i+=2)
-#define ADD_CHAR(c) {buffer[j] = (c); if (buffer[j] < ' ') buffer[j] = 0xb0; j++;}
+#define ADD_CHAR(c) buffer[j++] = (c) ? '.' : (c)
 	    ADD_CHAR(c[i]);
 	for(i=14;i<26 && c[i] && c[i]!=0xff;i+=2)
 	    ADD_CHAR(c[i]);
@@ -1271,7 +1271,7 @@ static void print_direntry(const direntry_t* direntry)
 
 static void print_mapping(const mapping_t* mapping)
 {
-    fprintf(stderr, "mapping (0x%x): begin, end = %d, %d, dir_index = %d, first_mapping_index = %d, name = %s, mode = 0x%x, " , (int)mapping, mapping->begin, mapping->end, mapping->dir_index, mapping->first_mapping_index, mapping->path, mapping->mode);
+    fprintf(stderr, "mapping (%p): begin, end = %d, %d, dir_index = %d, first_mapping_index = %d, name = %s, mode = 0x%x, " , mapping, mapping->begin, mapping->end, mapping->dir_index, mapping->first_mapping_index, mapping->path, mapping->mode);
     if (mapping->mode & MODE_DIRECTORY)
 	fprintf(stderr, "parent_mapping_index = %d, first_dir_index = %d\n", mapping->info.dir.parent_mapping_index, mapping->info.dir.first_dir_index);
     else
@@ -2843,7 +2843,7 @@ static void checkpoint(void) {
     return;
     /* avoid compiler warnings: */
     hexdump(NULL, 100);
-    remove_mapping(vvv, NULL);
+    remove_mapping(vvv, 0);
     print_mapping(NULL);
     print_direntry(NULL);
 }
-- 
1.6.0.4.1189.g8876f

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Andreas Schwab]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +#define ADD_CHAR(c) buffer[j++] = (c) ? '.' : (c)

isprint(c) perhaps?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Johannes Schindelin]

Hi,


Please Cc: me, I only did not miss your message because it is a slow 
Sunday morning (_and_ I am not in a deep hacking session).



On Sun, 14 Dec 2008, Andreas Schwab wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +#define ADD_CHAR(c) buffer[j++] = (c) ? '.' : (c)

Oops.  This is obviously wrong, and should read

#define ADD_CHAR(c) buffer[j++] = (c) < ' ' ? '.' : (c)

> isprint(c) perhaps?

I vote against using isprint(c).  The code is simple as it is, otherwise 
you'd have to look up what isprint() does, _and_ rely on isprint() being 
present.

I know, there are many people on this list who are all too happy to rely 
on new-fangled C99 stuff or lock themselves (and others) in with GCCisms. 
Peronally, though, I find it utterly stupid to do that unless you really 
have to.

Ciao,
Dscho

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Erik de Castro Lopo]

Johannes Schindelin wrote:

> I vote against using isprint(c).  The code is simple as it is, otherwise 
> you'd have to look up what isprint() does, _and_ rely on isprint() being 
> present.
> 
> I know, there are many people on this list who are all too happy to rely 
> on new-fangled C99 stuff or lock themselves (and others) in with GCCisms. 
> Peronally, though, I find it utterly stupid to do that unless you really 
> have to.

isprint is part of C89 and any compiler less than 20 years old should
support it.

Erik
-- 
-----------------------------------------------------------------
Erik de Castro Lopo
-----------------------------------------------------------------
Complex problems have simple easy to understand wrong answers.

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Johannes Schindelin]

Hi,

Please Cc: me if you reply directly to me.

On Sun, 14 Dec 2008, Erik de Castro Lopo wrote:

> Johannes Schindelin wrote:
> 
> > I vote against using isprint(c).  The code is simple as it is, 
> > otherwise you'd have to look up what isprint() does, _and_ rely on 
> > isprint() being present.
> > 
> > I know, there are many people on this list who are all too happy to 
> > rely on new-fangled C99 stuff or lock themselves (and others) in with 
> > GCCisms. Peronally, though, I find it utterly stupid to do that unless 
> > you really have to.
> 
> isprint is part of C89 and any compiler less than 20 years old should
> support it.

Okay, so it is C89.  So what does it do exactly?  Yes, exactly you have to 
look it up.  Nice job.

Whatever,
Dscho

P.S.: oh, so now the umlauts are not even printed when you are on an 
ISO-8859-1 console?  Nice job, again.

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Blue Swirl]

On 12/14/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
>  On Sun, 14 Dec 2008, Blue Swirl wrote:
>
>  > On 12/14/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>  >
>
> > >  On Sun, 14 Dec 2008, Blue Swirl wrote:
>  > >
>  > >  > Revision: 6023
>  > >  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6023
>  > >  > Author:   blueswir1
>  > >  > Date:     2008-12-14 09:30:41 +0000 (Sun, 14 Dec 2008)
>  > >  >
>  > >  > Log Message:
>  > >  > -----------
>  > >  > Use a hex value instead of possibly ambiguous 8 bit character
>  > >
>  > >
>  > > /me is curious: how could buffer[j] = '\xb0' be ambiguous when buffer is
>  > >  of type char *?  It's not as if C did UTF-8 conversion with chars.
>  >
>  > The diff does not show it properly, there was a 8 bit character between
>  > the apostrophes, not \xb0.
>
>
> Right.
>
>
>  > One day some compiler might want to parse the source text as UTF-8, then
>  > byte B0 and apostrophe after it could decode to something different with
>  > mysterious side effects.
>
>
> This will not be the case unless sizeof(char) will be anything else than
>  1.  Which will be, uhm, never.

Sizeof(char) has nothing to do with this. But if 0xb0 is invalid in
UTF-8, then we have nothing to worry, at least until the next
invention comes along.

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Blue Swirl]

On 12/14/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
>
>  Please Cc: me, I only did not miss your message because it is a slow
>  Sunday morning (_and_ I am not in a deep hacking session).
>
>
>
>
>  On Sun, 14 Dec 2008, Andreas Schwab wrote:
>
>  > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>  >
>  > > +#define ADD_CHAR(c) buffer[j++] = (c) ? '.' : (c)
>
>
> Oops.  This is obviously wrong, and should read
>
>
>  #define ADD_CHAR(c) buffer[j++] = (c) < ' ' ? '.' : (c)
>
>  > isprint(c) perhaps?
>
>
> I vote against using isprint(c).  The code is simple as it is, otherwise
>  you'd have to look up what isprint() does, _and_ rely on isprint() being
>  present.

I agree that we should not use plain old isprint, but instead the
highly advanced, omnipresent qemu_isprint which has none of the
problems of what isprint may have. ;-)

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[M. Warner Losh]

In message: <20081214224559.c54b844a.mle+tools@mega-nerd.com>
            Erik de Castro Lopo <mle+tools@mega-nerd.com> writes:
: Johannes Schindelin wrote:
: 
: > I vote against using isprint(c).  The code is simple as it is, otherwise 
: > you'd have to look up what isprint() does, _and_ rely on isprint() being 
: > present.
: > 
: > I know, there are many people on this list who are all too happy to rely 
: > on new-fangled C99 stuff or lock themselves (and others) in with GCCisms. 
: > Peronally, though, I find it utterly stupid to do that unless you really 
: > have to.
: 
: isprint is part of C89 and any compiler less than 20 years old should
: support it.

isprint() also was in many K&R compilers, but that was system
dependent.  The VAX I used in college in 1985 had it, as did the 'C'
compiler for my DEC Rainbow and the TOPS-20 C compiler written at my
university...

Warner

Re: [Qemu-devel] [6023] Use a hex value instead of possibly ambiguous 8 bit character

[Jamie Lokier]

Johannes Schindelin wrote:
> > isprint is part of C89 and any compiler less than 20 years old should
> > support it.
> 
> Okay, so it is C89.  So what does it do exactly?  Yes, exactly you have to 
> look it up.  Nice job.
> 
> Whatever,
> Dscho
> 
> P.S.: oh, so now the umlauts are not even printed when you are on an 
> ISO-8859-1 console?  Nice job, again.

isprint should return true for umlaut characters on an ISO-8859-1
console, provided the locale ($LANG, $LC_CTYPE) matches the console.

The question is whether you want QEMU's debugging output to depend on
the current locale, or be the same for everyone.  If the former,
isprint is quite appropriate.  If the latter, restricting to ASCII is
appropriate.

-- Jamie