Re: [PATCH v2] kernelbase: Implement SetCurrentConsoleFontEx

[Jacek Caban]

Hi Hugh,

On 2/8/22 13:38, Hugh McMaster wrote:
> Signed-off-by: Hugh McMaster
>
> Changes in this version:
> - Move struct condrv_output_info_params_font to SetCurrentConsoleFontEx
> as anonymous struct.
> - ioctl size only contains meaningful bytes.
> - Calculate face_name string size in conhost.
> - Manage invalid font height, weight and face name parameters.

It looks mostly good to me, some minor comments bellow.
> diff --git a/dlls/kernelbase/console.c b/dlls/kernelbase/console.c
> index a7eeb439232..5f64fc90fa3 100644
> --- a/dlls/kernelbase/console.c
> +++ b/dlls/kernelbase/console.c
> @@ -1303,6 +1303,46 @@ BOOL WINAPI DECLSPEC_HOTPATCH SetConsoleWindowInfo( HANDLE handle, BOOL absolute
>   }
>
> +/******************************************************************************
> -
> - SetCurrentConsoleFontEx   (kernelbase.@)
> - */
>
> +BOOL WINAPI SetCurrentConsoleFontEx(HANDLE hConsole, BOOL maxwindow, CONSOLE_FONT_INFOEX *cfix)
> +{
> - struct
> - {
> -
>    struct condrv_output_info_params params;
> -
>    WCHAR face_name[LF_FACESIZE];
> - } data;
> -
> - WCHAR *p = cfix->FaceName;
> - int len = 0;
> - DWORD size;
> -
> - TRACE( "(%p %d %p)\n", hConsole, maxwindow, cfix );
> -
> - if (cfix->cbSize != sizeof(CONSOLE_FONT_INFOEX))
> - {
> -
>    SetLastError(ERROR_INVALID_PARAMETER);
> -
>    return FALSE;
> - }
> -
> - data.params.mask = SET_CONSOLE_OUTPUT_INFO_FONT;
> -
> - data.params.info.font_width  = cfix->dwFontSize.X;
> - data.params.info.font_height = cfix->dwFontSize.Y;
> - data.params.info.font_pitch_family = cfix->FontFamily;
> - data.params.info.font_weight = cfix->FontWeight;
> -
> - while (*p && len < LF_FACESIZE) { p++; len++; }
> - size = len * sizeof(WCHAR);
> -
> - memcpy( data.face_name, cfix->FaceName, size );

This could be simplified to:

size = wcsnlen( info->FaceName, LF_FACESIZE - 1 ) * sizeof(WCHAR);
> @@ -1917,6 +1917,24 @@ static NTSTATUS set_output_info( struct screen_buffer *screen_buffer,
>           screen_buffer->max_width  = info->max_width;
>           screen_buffer->max_height = info->max_height;
>       }
> - if (params->mask & SET_CONSOLE_OUTPUT_INFO_FONT)
> - {
> -
>    WCHAR *face_name = (WCHAR *)(params + 1);
> -
>    size_t face_name_size = in_size - sizeof(*params);
> -
>    unsigned int height = info->font_height;
> -
>    unsigned int weight = FW_NORMAL;
> -
> -
>    if (!*face_name)

You potentially check uninitialized bytes here, I think you meant 
something like:

if (face_name_size)

Thanks,

Jacek