Re: A possible small tiny bug in win32 FULL_DEBUG build and some questions

[Eli Zaretskii <eliz@gnu.org>]

> From: arthur miller <arthur.miller@live.com>
> Date: Thu, 2 Jan 2025 15:59:32 +0000
> 
> Tiny bug: line 2447 in w32proc.c should probably read: cp->child_procs; not
> cp-child_procs.

Why do you think so?  child_procs is not a member of 'struct
_child_process', as you can see in w32.h, so cp->child_procs will not
compile.  'cp-child_procs' yields the ordinal number of the child
process in the child_procs[] array, which is what that debugging
printf wants to produce.

> Possible tiny opt/simplification: SetConsoleCP & SetConsoleOutputCP
> already do check for validity themselves (as far as I have tested with invalid
> codepages), so it is possible to simplify those two:
> 
> DEFUN ("w32-set-console-codepage", Fw32_set_console_codepage,
>        Sw32_set_console_codepage, 1, 1, 0,
>        doc: /* Make Windows codepage CP be the codepage for Emacs tty keyboard input.
> This codepage setting affects keyboard input in tty mode.
> If successful, the new CP is returned, otherwise nil.  */)
>   (Lisp_Object cp)
> {
>   CHECK_FIXNUM (cp);
> 
>   if (SetConsoleCP (XFIXNUM (cp)) != 0)
>     return make_fixnum (GetConsoleCP ());
>   return Qnil;
> }
> 
> DEFUN ("w32-set-console-output-codepage", Fw32_set_console_output_codepage,
>        Sw32_set_console_output_codepage, 1, 1, 0,
>        doc: /* Make Windows codepage CP be the codepage for Emacs console output.
> This codepage setting affects display in tty mode.
> If successful, the new CP is returned, otherwise nil.  */)
>   (Lisp_Object cp)
> {
>   CHECK_FIXNUM (cp);
> 
>   if (SetConsoleOutputCP (XFIXNUM (cp)) != 0)
>     return make_fixnum (GetConsoleOutputCP ());
>   return Qnil;
> }
> 
> Msdn (or whatever they call it now) docs says it returns 0 if it does not
> succeed. I have tested with few invalid numbers for CPs and those functions seem
> to work fine for me. I am not expert on win32, perhaps there is something I miss
> there? Nothing important, but if you want I can give you a patch.

We generally prefer not to call Win32 APIs with invalid parameters, if
that could be avoided, because doing that could cause an abort.  So I
don't think this optimization is worth the risk.

> Does GC-protected variables, those delcared as "staticpro", works the way I
> think they work: GC never collects them?

Yes.

> If I enum all codepages with w32-get-valid-codepages, than the list will forever
> stay alive?

Yes.  But its value could change if you call the function again, in
which case the old value will be GC'ed at some point.

> Even if the result from the function is never assigned to a variable
> int the user code?

Yes, even so.

> I don't see any usage of this function, neither in C code nor
> in Lisp (I grepped through); so I guess it is just eventually only called from
> the external packages or just interactively by the user. Is it worth to copy
> the returned list into a local list and return that so that the global list can
> be freed so not to leak that list in the case the user does not want to save
> that list anyway?

I don't understand where did you see a leak, and what kind of a leak
is that, please elaborate.

> Another question: can list of valid codepages on the system change during the
> runtime?

I suspect yes, if the user installs additional codepages.  But I don't
really know.

> Would it be OK to check if the list is alreay built, and just return
> it, instead of setting the list to nil on each run and re-computing it? I guess
> it is not something that changes often? I don't know really how it works on the
> system level, so just a question.

I don't see why risk the possibility that the list changes behind our
back.  What important advantages will we gain?

> Also, is it important that this list is in some particular order? Why reversing
> it before returning it?

I guess we want to return the list in the order the OS enumerates
them, not in the reverse order.  Again, why is this important?  I
don't think this function is likely to be called inside tight loops,
is it?