Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Bill Davidsen]

Andries Brouwer wrote:
>>>Shouldn't we be using "size_t" for unsigned int
> 
> 
>>You might be right. I was just being consistent with the other definitions.
> 
> 
> These are character positions on a screen.
> When did you last see a console in text mode with a line length
> of more than 2^31 ?
> 
> If you go for a minimal patch then you should replace "char"
> in one or two places by "unsigned char" and that is all.

Are these screen positions or offsets into the string for the line? The 
reason I ask is that with a 2-byte character set no char is going to do 
the latter reliably, not only do people run 132 col mode with vt100 
windows, just hitting the "full screen" button with most WM will give 
you >127 columns.

-- 
bill davidsen <davidsen@tmr.com>
   CTO TMR Associates, Inc
   Doing interesting things with small computers since 1979

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]

Hi.

Since we discussed this, I've produced a cut down patch and sent it to
my wide-console users. I can confirm that it is all that is needed. I'll
be posting a new patch for Andrew shortly.

Regards,

Nigel

On Sat, 2004-01-17 at 07:41, Bill Davidsen wrote:
> Andries Brouwer wrote:
> >>>Shouldn't we be using "size_t" for unsigned int
> > 
> > 
> >>You might be right. I was just being consistent with the other definitions.
> > 
> > 
> > These are character positions on a screen.
> > When did you last see a console in text mode with a line length
> > of more than 2^31 ?
> > 
> > If you go for a minimal patch then you should replace "char"
> > in one or two places by "unsigned char" and that is all.
> 
> Are these screen positions or offsets into the string for the line? The 
> reason I ask is that with a 2-byte character set no char is going to do 
> the latter reliably, not only do people run 132 col mode with vt100 
> windows, just hitting the "full screen" button with most WM will give 
> you >127 columns.
-- 
My work on Software Suspend is graciously brought to you by
LinuxFund.org.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Pavel Machek]

Hi!

> You might be right. I was just being consistent with the other
> definitions. Andrew? Benjamin?

I believe size_t is meant for file size, memory size and similar. You
should stuck with int.
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Edgar Toernig]

Nigel Cunningham wrote:
>
> Andries Brouwer wrote:
>
> > When did you last see a console in text mode with a line length
> > of more than 2^31 ?
> >
> > If you go for a minimal patch then you should replace "char"
> > in one or two places by "unsigned char" and that is all.
>
> Of course you're right about 2^31 columns, but the rest of the code used
> unsigned ints as well, not because it expects 2^31 columns, but because
> (if I understand the code right), the numbers can be part of escape
> sequences... I'm looking at csi_m in vt.c.

Yes, they may be part of escape sequences and that's why your patch is
wrong.  See the comment above gotoxy:

    /*
     * gotoxy() must verify all boundaries, because the arguments
     * might also be negative. If the given position is out of
     * bounds, the cursor is placed at the nearest margin.
    */

If you make the arguments unsigned it will choose the wrong
margin for negative values.  That will i.e. happen if you send
the sequence to move the cursor 10 spaces left when it is only
in column 5.

Afaics, the only thing that needs fixing is putconsxy:

 void putconsxy(int currcons, char *p)
 {
-        gotoxy(currcons, p[0], p[1]);
+        gotoxy(currcons, (unsigned char)p[0], (unsigned char)p[1]);
         set_cursor(currcons);
 }

Ciao, ET.

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Andrew Morton]

Nigel Cunningham <ncunningham@users.sourceforge.net> wrote:
>
> X-Mailer: Ximian Evolution 1.4.4-8mdk

Gack.  My MUA (sylpheed) is unable to decrypt the plain text after whatever
it is that Evolution did to it.  Please see if you can get Evolution to
play more nicely with plain text, or use attachments.

I fixed that one up by hand.

Thanks.

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Andrew Morton]

Nigel Cunningham <ncunningham@users.sourceforge.net> wrote:
>
> This patch makes console X and Y coordinates unsigned, rather than
> signed. Issues with wide (> 128 char?) consoles, seen when developing
> Software Suspend's 'nice display' are thus fixed. A brief examination of
> related code showed that this use of signed variables was the exception
> rather than the rule.

You'll probably find that all you needed to do was to convert the chars
over to unsigned, not the integers.

But whatever.

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]

Hi.

Of course you're right about 2^31 columns, but the rest of the code used
unsigned ints as well, not because it expects 2^31 columns, but because
(if I understand the code right), the numbers can be part of escape
sequences... I'm looking at csi_m in vt.c.

Regards,

Nigel

On Sat, 2004-01-10 at 09:33, Andries Brouwer wrote:
> >> Shouldn't we be using "size_t" for unsigned int
> 
> > You might be right. I was just being consistent with the other definitions.
> 
> These are character positions on a screen.
> When did you last see a console in text mode with a line length
> of more than 2^31 ?
> 
> If you go for a minimal patch then you should replace "char"
> in one or two places by "unsigned char" and that is all.
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
My work on Software Suspend is graciously brought to you by
LinuxFund.org.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Andries Brouwer]

>> Shouldn't we be using "size_t" for unsigned int

> You might be right. I was just being consistent with the other definitions.

These are character positions on a screen.
When did you last see a console in text mode with a line length
of more than 2^31 ?

If you go for a minimal patch then you should replace "char"
in one or two places by "unsigned char" and that is all.

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

You might be right. I was just being consistent with the other
definitions. Andrew? Benjamin?

Regards,

Nigel

On Sat, 2004-01-10 at 08:22, Richard B. Johnson wrote:
> On Sat, 10 Jan 2004, Nigel Cunningham wrote:
> 
> > This patch makes console X and Y coordinates unsigned, rather than
> > signed. Issues with wide (> 128 char?) consoles, seen when developing
> > Software Suspend's 'nice display' are thus fixed. A brief examination of
> > related code showed that this use of signed variables was the exception
> > rather than the rule.
> >
> > Regards,
> >
> > Nigel
> [SNIPPED...]
> 
> Question: Shouldn't we be using "size_t" for unsigned int, and
> "ssize_t" for signed? If the "ints" are going to be changed,
> they probably should be changed only once. As I recall, size_t
> was the largest unsigned int that would fit into a register and
> ssize_t was the largest signed int that would fit.
> 
> Cheers,
> 
> Dick Johnson
> Penguin : Linux version 2.4.22 on an i686 machine (797.90 BogoMips).
>             Note 96.31% of all statistics are fiction.
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
My work on Software Suspend is graciously brought to you by
LinuxFund.org.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Richard B. Johnson]

On Sat, 10 Jan 2004, Nigel Cunningham wrote:

> This patch makes console X and Y coordinates unsigned, rather than
> signed. Issues with wide (> 128 char?) consoles, seen when developing
> Software Suspend's 'nice display' are thus fixed. A brief examination of
> related code showed that this use of signed variables was the exception
> rather than the rule.
>
> Regards,
>
> Nigel
[SNIPPED...]

Question: Shouldn't we be using "size_t" for unsigned int, and
"ssize_t" for signed? If the "ints" are going to be changed,
they probably should be changed only once. As I recall, size_t
was the largest unsigned int that would fit into a register and
ssize_t was the largest signed int that would fit.

Cheers,

Dick Johnson
Penguin : Linux version 2.4.22 on an i686 machine (797.90 BogoMips).
            Note 96.31% of all statistics are fiction.

PATCH 1/2: Make gotoxy & siblings use unsigned variables

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]

This patch makes console X and Y coordinates unsigned, rather than
signed. Issues with wide (> 128 char?) consoles, seen when developing
Software Suspend's 'nice display' are thus fixed. A brief examination of
related code showed that this use of signed variables was the exception
rather than the rule.

Regards,

Nigel

diff -ruN linux-2.6.1-rc3/drivers/char/vt.c console-patch-1/drivers/char/vt.c
--- linux-2.6.1-rc3/drivers/char/vt.c	2004-01-09 14:24:45.000000000 +1300
+++ console-patch-1/drivers/char/vt.c	2004-01-10 07:17:08.000000000 +1300
@@ -149,7 +149,7 @@
 static void vc_init(unsigned int console, unsigned int rows,
 		    unsigned int cols, int do_clear);
 static void blank_screen(unsigned long dummy);
-static void gotoxy(int currcons, int new_x, int new_y);
+static void gotoxy(int currcons, unsigned int new_x, unsigned int new_y);
 static void save_cur(int currcons);
 static void reset_terminal(int currcons, int do_clear);
 static void con_flush_chars(struct tty_struct *tty);
@@ -859,9 +859,9 @@
  * might also be negative. If the given position is out of
  * bounds, the cursor is placed at the nearest margin.
  */
-static void gotoxy(int currcons, int new_x, int new_y)
+static void gotoxy(int currcons, unsigned int new_x, unsigned int new_y)
 {
-	int min_y, max_y;
+	unsigned int min_y, max_y;
 
 	if (new_x < 0)
 		x = 0;
@@ -888,7 +888,7 @@
 }
 
 /* for absolute user moves, when decom is set */
-static void gotoxay(int currcons, int new_x, int new_y)
+static void gotoxay(int currcons, unsigned int new_x, unsigned int new_y)
 {
 	gotoxy(currcons, new_x, decom ? (top+new_y) : new_y);
 }
@@ -2996,13 +2996,13 @@
 	return screenpos(currcons, 2 * w_offset, viewed);
 }
 
-void getconsxy(int currcons, char *p)
+void getconsxy(int currcons, unsigned char *p)
 {
 	p[0] = x;
 	p[1] = y;
 }
 
-void putconsxy(int currcons, char *p)
+void putconsxy(int currcons, unsigned char *p)
 {
 	gotoxy(currcons, p[0], p[1]);
 	set_cursor(currcons);
diff -ruN linux-2.6.1-rc3/include/linux/selection.h console-patch-1/include/linux/selection.h
--- linux-2.6.1-rc3/include/linux/selection.h	2004-01-09 14:22:25.000000000 +1300
+++ console-patch-1/include/linux/selection.h	2004-01-10 07:17:08.000000000 +1300
@@ -36,8 +36,8 @@
 extern void complement_pos(int currcons, int offset);
 extern void invert_screen(int currcons, int offset, int count, int shift);
 
-extern void getconsxy(int currcons, char *p);
-extern void putconsxy(int currcons, char *p);
+extern void getconsxy(int currcons, unsigned char *p);
+extern void putconsxy(int currcons, unsigned char *p);
 
 extern u16 vcs_scr_readw(int currcons, const u16 *org);
 extern void vcs_scr_writew(int currcons, u16 val, u16 *org);

-- 
My work on Software Suspend is graciously brought to you by
LinuxFund.org.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]