Re: [PATCH v2] conhost: Use font face length as number of characters, not size

[Jacek Caban]

On 8/13/21 3:52 PM, Hugh McMaster wrote:
> Hi Jacek,
>
> On Fri, 13 Aug 2021 at 23:11, Jacek Caban wrote:
> > On 8/12/21 3:18 PM, Hugh McMaster wrote:
> > > -        if (!(screen_buffer->font.face_name = malloc( screen_buffer->font.face_len ))) return NULL;
> > > -        memcpy( screen_buffer->font.face_name, console->active->font.face_name,
> > > -                screen_buffer->font.face_len );
> > > +        if (screen_buffer->font.face_len)
> > > +        {
> > > +            screen_buffer->font.face_name = malloc((screen_buffer->font.face_len + 1) * sizeof(WCHAR));
> > > +            if (!screen_buffer->font.face_name) return NULL;
> > > +            lstrcpyW(screen_buffer->font.face_name, console->active->font.face_name);
> >
> > This does not only change the meaning of length, but also
> > null-terminates the string. Is there any reason for that? It doesn't
> > make much sense to both store the length and null-terminate string at
> > the same time.
> > The FaceName value is stored null-terminated in the registry. It's
> > also null-terminated in the LOGFONTW structure, when copied to the
> > config and in dialogs.
>
> We can probably get rid of face_len though.

While in this case it doesn't matter all that much, I don't see a point 
in having it null-terminated. Storing the length separately has the 
advantage that we don't need to recalculate it whenever we need it and 
it's easy enough to null-terminate it when copying to structs that need it.


Thanks,

Jacek