[Patch 1/2] drivers/char/vt.c: remove unnecessary code
In the palette escape sequence, setting the elems of par[] at 0 is not needed,
since its values are every time overwritten in this case.
Signed-off-by: Emmanuel Colbus <[EMAIL PROTECTED]>
--- old/drivers/char/vt.c 2004-12-24 22:35:25.0 +0100
+++ new/drivers/char/vt.c 2005-02-28 12:51:43.910651512 +0100
@@ -1626,8 +1626,6 @@
return;
case ESnonstd:
if (c=='P') { /* palette escape sequence */
- for (npar=0; nparhttp://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 3/2] drivers/char/vt.c: remove unnecessary code
We could change an affectation into an incrementation by this patch, and,
so far I know, incrementing is quicker than or as quick as setting
a variable (depends on the architecture).
Please _don't_ apply this, but tell me what you think about it.
Note that npar is unsigned.
Signed-off-by: Emmanuel Colbus <[EMAIL PROTECTED]>
--- old/drivers/char/vt.c 2004-12-24 22:35:25.0 +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 < NPAR; npar--)
par[npar] = 0;
+ npar++;
- npar = 0;
vc_state = ESgetpars;
if (c == '[') { /* Function key */
vc_state=ESfunckey;
--
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[patch 2/2] drivers/chat/vt.c: remove unnecessary code
Avoid changing the state of the console two times in some cases.
Signed-off-by: Emmanuel Colbus <[EMAIL PROTECTED]>
--- old/drivers/char/vt.c 2004-12-24 22:35:25.0 +0100
+++ new/drivers/char/vt.c 2005-02-28 12:56:46.154311486 +0100
@@ -1571,7 +1571,6 @@
}
switch(vc_state) {
case ESesc:
- vc_state = ESnormal;
switch (c) {
case '[':
vc_state = ESsquare;
@@ -1585,25 +1584,25 @@
case 'E':
cr(currcons);
lf(currcons);
- return;
+ break;
case 'M':
ri(currcons);
- return;
+ break;
case 'D':
lf(currcons);
- return;
+ break;
case 'H':
tab_stop[x >> 5] |= (1 << (x & 31));
- return;
+ break;
case 'Z':
respond_ID(tty);
- return;
+ break;
case '7':
save_cur(currcons);
- return;
+ break;
case '8':
restore_cur(currcons);
- return;
+ break;
case '(':
vc_state = ESsetG0;
return;
@@ -1615,14 +1614,15 @@
return;
case 'c':
reset_terminal(currcons,1);
- return;
+ break;
case '>': /* Numeric keypad */
clr_kbd(kbdapplic);
- return;
+ break;
case '=': /* Appl. keypad */
set_kbd(kbdapplic);
- return;
+ /* Here, we don't need any break; */
}
+ vc_state = ESnormal;
return;
case ESnonstd:
if (c=='P') { /* palette escape sequence */
--
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 3/2] drivers/char/vt.c: remove unnecessary code
>On Mon, Feb 28, 2005 at 01:57:59PM +0100, [EMAIL PROTECTED] wrote:
>> Please _don't_ apply this, but tell me what you think about it.
>It's broken. 8)
>> --- old/drivers/char/vt.c 2004-12-24 22:35:25.0 +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 < NPAR; npar--)
>How many times do you want this for loop to run?
NPAR times :-). As I stated, npar is unsigned.
See :
$cat x.c
#include
#define NPAR 5
int main(void){
unsigned int i;
int j=0;
for(i=NPAR-1;ihttp://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 3/2] drivers/char/vt.c: remove unnecessary code
Surlignage Russell King <[EMAIL PROTECTED]>:
> On Mon, Feb 28, 2005 at 02:13:57PM +0100, [EMAIL PROTECTED] 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.0 +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.0 +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 intvc_state; /* Escape sequence parser state
*/
- unsigned intvc_npar,vc_par[NPAR]; /* Parameters of current escape
sequence */
+ unsigned intvc_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 intvc_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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 3/2] drivers/char/vt.c: remove unnecessary code
>> - for(npar = 0 ; npar < NPAR ; npar++)
>> + for(npar = NPAR - 1; npar >= 0; npar--)
>> par[npar] = 0;
>if you really want to clean this up..
Well, actually, I was not myself entirely convinced about it... This is the
reason for I wrote "please _don't_ apply this, but tell me what you think about
it.".
>why not use memset() instead ?
Because I simply didn't thought to it :-) .
Hey, that makes fully sense! So far I know, memset() is quicker than
(or as quick as) a loop, and it remains fully readable (in my opinion :).
Well, such a patch would be :
--- drivers/char/vt.c 2004-12-24 22:35:25.0 +0100
+++ drivers/char/vt2.c 2005-02-28 15:55:11.782717810 +0100
@@ -1655,8 +1655,8 @@
vc_state = ESnormal;
return;
case ESsquare:
- for(npar = 0 ; npar < NPAR ; npar++)
- par[npar] = 0;
+ /* Setting par[]'s elems at 0. */
+ memset(par, 0, NPAR*sizeof(unsigned int));
npar = 0;
vc_state = ESgetpars;
if (c == '[') { /* Function key */
Thank you for the suggestion.
What do you think of this one?
(Please note that I'm not trying to get a patch for it _by force_ into the
kernel. If it's a bad idea, let's let thing like they currently are,
the current loop just works.)
cu
--
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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2/2] drivers/chat/vt.c: remove unnecessary code
>On Mon, Feb 28, 2005 at 01:55:28PM +0100, [EMAIL PROTECTED] wrote: > >> Avoid changing the state of the console two times in some cases. > >A bad change for several reasons. > >(i) more object code is generated >(ii) the code is slower >(iii) you change something > >Straight line code is cheap, jumps are expensive. >Replacing an assignment by a jump is not an improvement. I didn't thought to that point... You're right, this patch was a bad idea :(. Sorry about this error... -- 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.4]drivers/char/console.c: check if caller is proprietary of the current console
This patch adds a verification that the calling process is writing to the current console, in the DEC alignment screen test. (The bug can be observed by typing : $chvt 2 && sleep 1 && echo -e "\033#8" in vt1.) Signed-off-by: Emmanuel Colbus <[EMAIL PROTECTED]> --- This patch was already sent on: - 04 Jan 2005 --- old/drivers/char/console.c 2005-01-03 15:51:22.0 +0100 +++ patched/drivers/char/console.c 2005-01-03 15:31:45.0 +0100 @@ -1781,7 +1781,7 @@ csi_J(currcons, 2); video_erase_char = (video_erase_char & 0xff00) | ' '; - do_update_region(currcons, origin,screenbuf_size/2); + update_region(currcons, origin, screenbuf_size/2); } return; case ESsetG0: -- 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.2]drivers/char/console.c: check if caller is proprietary of the current console
This patch adds a verification that the calling process is writing to the current console, in the DEC alignment screen test. Signed-off-by: Emmanuel Colbus <[EMAIL PROTECTED]> --- This patch was already sent on: - 04 Jan 2005 --- old/drivers/char/console.c Mon Sep 16 18:26:11 2002 +++ patched/drivers/char/console.c Tue Jan 4 09:54:58 2005 @@ -1742,7 +1742,7 @@ csi_J(currcons, 2); video_erase_char = (video_erase_char & 0xff00) | ' '; - do_update_region(currcons, origin, screenbuf_size/2); + update_region(currcons, origin, screenbuf_size/2); } return; case ESsetG0: -- 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
