On Mon, 25 Jun 2018 17:56:21 +0200, Mattias Engdegard wrote:
> Fix broken example code in the vcs.4 man page:
>
> - use of wrong variable (attrib, which is uninitialised, instead of s)
> - variable ch too narrow
> - printing a font char index with %c, as if it were ASCII (it's not)
> - removing the high font bit while changing the background colour
>
> Also be friendly and use SEEK_* instead of numbers.
>
>
> diff --git a/man4/vcs.4 b/man4/vcs.4
> index aebe8cfda..c8b8cf361 100644
> --- a/man4/vcs.4
> +++ b/man4/vcs.4
> @@ -140,7 +140,8 @@ main(void)
> struct {unsigned char lines, cols, x, y;} scrn;
> unsigned short s;
> unsigned short mask;
> - unsigned char ch, attrib;
> + unsigned char attrib, a;
> + int ch;
>
> fd = open(console, O_RDWR);
> if (fd < 0) {
> @@ -158,16 +159,16 @@ main(void)
> exit(EXIT_FAILURE);
> }
> (void) read(fd, &scrn, 4);
> - (void) lseek(fd, 4 + 2*(scrn.y*scrn.cols + scrn.x), 0);
> + (void) lseek(fd, 4 + 2*(scrn.y*scrn.cols + scrn.x), SEEK_SET);
> (void) read(fd, &s, 2);
> ch = s & 0xff;
> - if (attrib & mask)
> + if (s & mask)
> ch |= 0x100;
> attrib = ((s & ~mask) >> 8);
> - printf("ch=\(aq%c\(aq attrib=0x%02x\\n", ch, attrib);
> - attrib ^= 0x10;
> - (void) lseek(fd, \-1, 1);
> - (void) write(fd, &attrib, 1);
> + printf("ch=0x%03x attrib=0x%02x\\n", ch, attrib);
> + a = (s >> 8) ^ 0x10;
> + (void) lseek(fd, \-1, SEEK_CUR);
> + (void) write(fd, &a, 1);
> exit(EXIT_SUCCESS);
> }
> .EE
Ha! This is the erroneous code example that prompted me to overhaul
all of `vcs.4' (hopefully, I'll be submitting a patch soon), and
this work further prompted me to write the proposed new page:
byte.7
Message-ID: <88a3d23bc0b14bf89ea303ae82468f59-mfwitten@xxxxxxxxx>
https://www.spinics.net/lists/linux-man/msg13143.html
I'm actually a little surprised to see that someone else came across
it, too; I figured that `vcs.4' is, these days, a fairly forgotten
topic, but I guess I was wrong!
Anyway, putting aside the fact that this code example makes certain
assumptions about the sizes of data types (including a `short', as
not even POSIX constrains it to exactly 16 bits), there is still the
issue of endianness. Namely, the following works only on machines of
little-endian nature:
a = (s >> 8) ^ 0x10;
(void) lseek(fd, -1, SEEK_CUR);
(void) write(fd, &a, 1);
A more widely applicable example cannot merely step back one byte
and then replace what's there; instead, it must be aware of the byte
order in which the 2-byte character/attribute pair is stored. One
way to achieve the correct results is to construct a whole new
2-byte character/attribute pair, step back 2 bytes, and then write
that new pair in place. So, it could be something more like this:
s ^= (0x10 << 8);
(void) lseek(fd, -2, SEEK_CUR);
(void) write(fd, &s, 2);
To add a little clarity to our collision here, I think your patch
should be applied (perhaps with the endianness issue shored up), and
I'll plan to rebase my renovation of `vcs.4' on top of your patch.
Sincerely,
Michael Witten
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
