Re: [patch 3/2] drivers/char/vt.c: remove unnecessary code

From: colbuse
Date: Mon Feb 28 2005 - 09:18:08 EST


Surlignage Russell King <rmk+lkml@xxxxxxxxxxxxxxxx>:

> On Mon, Feb 28, 2005 at 02:13:57PM +0100, colbuse@xxxxxxxxxxxxxxx wrote:
> > NPAR times :-). As I stated, npar is unsigned.
>
> I think that's disgusting then - it isn't obvious what's going on, which
> leads to mistakes.
>
> For the sake of a micro-optimisation such as this, it's far more important
> that the code be readable and easily understandable.
>
> Others may disagree.
>

I agree :) . But, if we look to the code, we can notice that there is actually
no reason for npar to be unsigned. What do you think of this version?


--- old/drivers/char/vt.c 2004-12-24 22:35:25.000000000 +0100
+++ new/drivers/char/vt.c 2005-02-28 12:53:57.933256631 +0100
@@ -1655,9 +1655,9 @@
vc_state = ESnormal;
return;
case ESsquare:
- for(npar = 0 ; npar < NPAR ; npar++)
+ for(npar = NPAR - 1; npar >= 0; npar--)
par[npar] = 0;
+ npar++;
- npar = 0;
vc_state = ESgetpars;
if (c == '[') { /* Function key */
vc_state=ESfunckey;


--- old/include/linux/console_struct.h 2004-12-24 22:33:48.000000000 +0100
+++ old/include/linux/console_struct.h 2005-02-28 14:49:53.653616335 +0100
@@ -44,7 +44,8 @@
unsigned short vc_video_erase_char; /* Background erase character */
/* VT terminal data */
unsigned int vc_state; /* Escape sequence parser state */
- unsigned int vc_npar,vc_par[NPAR]; /* Parameters of current escape
sequence */
+ unsigned int vc_par[NPAR]; /* Parameters of current escape
sequence */
+ int vc_npar; /* Current position in vc_par */
struct tty_struct *vc_tty; /* TTY we are attached to */
/* mode flags */
unsigned int vc_charset : 1; /* Character set G0 / G1 */


--
Emmanuel Colbus
Club GNU/Linux
ENSIMAG - Departement telecoms


-------------------------------------------------
envoyé via Webmail/IMAG !

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/