linux-kernel - Re: [PATCH v2 1/2] console: Remove dummy con_font

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201102105228.GA1558395@PWN>
Date:   Mon, 2 Nov 2020 05:52:28 -0500
From:   Peilin Ye <yepeilin.cs@...il.com>
To:     Daniel Vetter <daniel@...ll.ch>, Jiri Slaby <jirislaby@...nel.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thomas Winischhofer <thomas@...ischhofer.net>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
        Nicolas Pitre <nico@...xnic.net>,
        "Gustavo A . R . Silva" <gustavoars@...nel.org>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        George Kennedy <george.kennedy@...cle.com>,
        Nathan Chancellor <natechancellor@...il.com>,
        Peter Rosin <peda@...ntia.se>, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-fbdev@...r.kernel.org
Subject: Re: [PATCH v2 1/2] console: Remove dummy con_font_op() callback
 implementations

On Mon, Nov 02, 2020 at 11:13:47AM +0100, Daniel Vetter wrote:
> On Mon, Nov 02, 2020 at 10:47:55AM +0100, Jiri Slaby wrote:
> > On 02. 11. 20, 10:36, Peilin Ye wrote:
> > > `struct console_font` is a UAPI structure, thus ideally should not be
> > > used for kernel internal abstraction. Remove some dummy .con_font_set,
> > > .con_font_default and .con_font_copy `struct consw` callback
> > > implementations, to make it cleaner.
> > 
> > ESEMANTIC_ERROR.
> > 
> > 1) What do you refer to with the last "it"?
> > 
> > 2) What's the purpose of mentioning struct console_font at all?
> > 
> > 3) Could you clarify whether you checked it is safe to remove the hooks?

I see. I will try to elaborate in future patches, thank you!

> > 4) All the hooks now return ENOSYS for both consoles (and not 0). Is this
> > intentional?
> > 
> > I know answers to the first 3 questions, but you need to elaborate a bit in
> > the commit log to connect those sentences. Esp. for people not dealing with
> > the code on a daily basis. Ad 4) I am not sure.
> 
> Yup the behaviour change from 4) needs to be called out. I think this
> should then also be done as part of the large patch series to remove the
> dummy functions from all console drivers.
> 
> I don't expect the errno change to cause trouble, and it's the more honest
> errno - changing fonts not supported is the truth. But if it is, we can
> patch that up appropriately when we get a regression report. That's kinda
> unavoidable with old crufty uapi like this one here.
> 
> Also a bikeshed: Additional information like the patch changelog or
> reasons why you do something is imo best to include in the commit message
> itself. It ends up looking a bit less tidy sometimes, but often there's
> crucial information in these parts that was accidentally left out from the
> commit message.

Sure, I will try to include sufficient information for one to easily
understand what I'm trying to do with a patch. Thank you,

Peilin