doesn't this look incorrect?

doesn't this look incorrect?

[Christopher Plewright]

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


See Lines  9733 to 9735: in emacs:  src/w32fns.c

  static int isdead = 0;

  if (isdead == 2)

This is apparently a mistake.  This sort of thing usually happens when refactoring gets screwed up.

Seems to have been there for a long time!?   Trying to guess what the original intention might have been here;

http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD


9723<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9723> /* The return code indicates key code size.  cpID is the codepage to
9724<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9724>    use for translation to Unicode; -1 means use the current console
9725<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9725>    input codepage.  */
9726<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9726> int
9727<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9727> w32_kbd_patch_key (KEY_EVENT_RECORD *event, int cpId)
9728<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9728> {
9729<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9729>   unsigned int key_code = event->wVirtualKeyCode;
9730<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9730>   unsigned int mods = event->dwControlKeyState;
9731<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9731>   BYTE keystate[256];
9732<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9732>   static BYTE ansi_code[4];
9733<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9733>   static int isdead = 0;
9734<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9734>
9735<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9735>   if (isdead == 2)
9736<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9736>     {
9737<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9737>       event->uChar.AsciiChar = ansi_code[2];
9738<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9738>       isdead = 0;
9739<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9739>       return 1;
9740<http://git.savannah.gnu.org/gitweb/?p=emacs.git;a=blob;f=src/w32fns.c;h=887e5a1f7b74550044c0be0d40fd0bebab684eb7;hb=HEAD#l9740>     }


[-- Attachment #2: Type: text/html, Size: 10931 bytes --]

Re: doesn't this look incorrect?

[Óscar Fuentes]

Christopher Plewright <chris@createng.com> writes:

> See Lines  9733 to 9735: in emacs:  src/w32fns.c
>
>   static int isdead = 0;
>
>   if (isdead == 2)
>
> This is apparently a mistake.

I don't think so. See what `static' means for a variable declaration
inside a function.

Re: doesn't this look incorrect?

[tomas]

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

On Mon, Nov 28, 2022 at 06:26:19AM +0100, Óscar Fuentes wrote:
> Christopher Plewright <chris@createng.com> writes:
> 
> > See Lines  9733 to 9735: in emacs:  src/w32fns.c
> >
> >   static int isdead = 0;
> >
> >   if (isdead == 2)
> >
> > This is apparently a mistake.
> 
> I don't think so. See what `static' means for a variable declaration
> inside a function.

Still it seems to me that Christopher has a point: the `if' doesn't
seem reachable from any other path, so `isdead' will be always zero
at that point?

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

RE: doesn't this look incorrect?

[Christopher Plewright]

Ahh, thanks, I missed that.  So, static means it's remembered out of its declared scope, and so its only initialized first time.

-----Original Message-----
From: Óscar Fuentes <ofv@wanadoo.es> 
Sent: Monday, November 28, 2022 4:26 PM
To: Christopher Plewright <chris@createng.com>
Cc: emacs-devel@gnu.org
Subject: Re: doesn't this look incorrect?

Christopher Plewright <chris@createng.com> writes:

> See Lines  9733 to 9735: in emacs:  src/w32fns.c
>
>   static int isdead = 0;
>
>   if (isdead == 2)
>
> This is apparently a mistake.

I don't think so. See what `static' means for a variable declaration inside a function.

Re: doesn't this look incorrect?

[Eli Zaretskii]

> Date: Mon, 28 Nov 2022 06:30:22 +0100
> From: <tomas@tuxteam.de>
> 
> > >   static int isdead = 0;
> > >
> > >   if (isdead == 2)
> > >
> > > This is apparently a mistake.
> > 
> > I don't think so. See what `static' means for a variable declaration
> > inside a function.
> 
> Still it seems to me that Christopher has a point: the `if' doesn't
> seem reachable from any other path, so `isdead' will be always zero
> at that point?

It will be "reachable" by the next call to this function.  The code below
the 'if' updates the variable, in several places.

Re: doesn't this look incorrect?

[tomas]

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

On Mon, Nov 28, 2022 at 02:19:45PM +0200, Eli Zaretskii wrote:
> > Date: Mon, 28 Nov 2022 06:30:22 +0100
> > From: <tomas@tuxteam.de>

[...]

> > Still it seems to me that Christopher has a point: the `if' doesn't
> > seem reachable from any other path [...]

> It will be "reachable" by the next call to this function.  The code below
> the 'if' updates the variable, in several places.

You're right, of course. Note to self: don't read C code before first
coffee had effect.

Sorry for the noise.

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]