[PATCH 1/6] conhost: Move console registry key path into 'struct console'

[Hugh McMaster]

Hi Jacek,

On Thu, 14 Apr 2022 at 04:29, Jacek Caban wrote:
>
> Hi Hugh,
>
> On 4/11/22 14:35, Hugh McMaster wrote:
> > Signed-off-by: Hugh McMaster<hugh.mcmaster at outlook.com>
> > ---
> >   programs/conhost/conhost.h |  1 +
> >   programs/conhost/window.c  | 15 +++++++--------
> >   2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h
> > index 8ca09bb80d0..2cb9515037a 100644
> > --- a/programs/conhost/conhost.h
> > +++ b/programs/conhost/conhost.h
> > @@ -77,6 +77,7 @@ struct console
> >       HANDLE                 server;              /* console server handle */
> >       unsigned int           mode;                /* input mode */
> >       struct screen_buffer  *active;              /* active screen buffer */
> > +    WCHAR                 *config_key;          /* registry key name for app-specific settings */
> >       int                    is_unix;             /* UNIX terminal mode */
> >       int                    use_relative_cursor; /* use relative cursor positionning */
> >       INPUT_RECORD          *records;             /* input records */
>
>
> Why do you need that change? It seems to me that when it's needed, we
> always have console_window struct available anyway. Similar question to
> patches 2, 3, and 4 of the series, it's not obvious to me what's the
> plan for them.

Patch 1. I know it doesn't make a difference to how we use or access
config_key, but moving it to struct console seems to be a more natural
location than struct console_window. Moving config_key to struct
console is also consistent with pointers to other app- and
window-specific members, such as struct screen_buffer *active and
struct console_window *window, both of which are accessed from struct
console. You'll note that struct console holds *title, history-related
members etc., while other active config sits in struct screen_buffer
and struct console_window.

Patch 2. Calls to RegOpenKey ultimately call RegOpenKeyEx (with the
broadest access), so it's better to just call RegOpenKeyEx directly.
There should also be a tiny speed improvement by reducing the number
of calls, but I haven't done any testing on that.

Patch 3. I should have added a commit message on this. On Windows,
HKCU\Console holds global console default settings, and
HKCU\Console\<app_path> holds any app-specific settings that differ
from the global settings. If there are no differences, the
app-specific key does not exist.

Wine currently stores all console settings in an app-specific key
after setting a default font on first init. Global console settings
are only saved in HKCU\Console if using the Default settings dialog
option, but this results in two sets of console settings in the
registry. This patch stores default settings in HKCU\Console on first
run. App-specific settings are then saved in the relevant subkey if
any specific settings are set by the user via the Properties dialog.
This change is required so we can eventually only save app-specific
settings that differ from the global settings, mirroring Windows.

Patch 4.  With this patch, I wanted to clean up the multiple calls to
RegCreateKey and save_registry_key, and avoid duplicate error
handling, when we could handle this code logic once. As we only ever
save to HKCU\Console or a subkey, it seems unnecessary to open
HKCU\Console via RegCreateKey and then open the subkey as well.

I don't how strongly you feel about these changes. From my
perspective, I think they're all nice improvements, but I do need
patch 3. I'm working on an additional patch so we only save different
settings to the app-specifc registry key, mirroring the behaviour on
Windows.

Thanks for seeking clarification. Happy to discuss further.

Hugh