Messages in this thread | | | Date | Wed, 15 Feb 2023 17:13:17 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH v2] tty: ttynull: implement console write |
| |
On Wed 2023-02-15 16:24:23, John Ogness wrote: > On 2023-02-15, Petr Mladek <pmladek@suse.com> wrote: > > That said, the current code is error-prone. The check for non-NULL > > con->write is too far from the caller. > > con->write is supposed to be immutable after registration, so having the > check "far from the caller" is not a real danger. > > console_emit_next_record() is the toplevel function responsible for > printing on a particular console so I think it is appropriate that the > check is made when determining if this function should be called. I also > think console_is_usable() is the proper place for the NULL check to > reside since that is the function that determines if printing is > allowed.
I agree that the current code is not that bad. But still, the call chain is:
+ console_flush_all() + console_emit_next_record() + call_console_driver() + con->write()
I could imagine another code that would call call_console_driver() or console_emit_next_record() directly. We might just hope that it would check console_is_usable() or con->write pointer before. I consider this error-prone.
Also, as you say, con->write is immutable. All real consoles have it defined. It does not make sense to check it again and again. I would leave console_is_usable() for checks that might really change at runtime.
> > I would prefer to make it more safe. For example, I would prevent > > registration of consoles without con->write callback in the first > > place. It would require, to implement the empty write() callback > > for ttynull console as done by this patch. > > I would prefer that we do not start encouraging dummy implementations. > If you insist on con->write having _some_ value other than NULL, then we > could define some macro with a special value (CONSOLE_NO_WRITE). But > then we have to check that value. IMHO, allowing NULL is not an issue.
ttynull is really special. It is a dummy driver and dummy callbacks are perfectly fine. IMHO, one dummy driver is enough. Ideally, the generic printk code should not need any special hacks to handle it.
IMHO, normal console drivers would be useless without write() callback. It sounds perfectly fine to reject useless console drivers at all. And it sounds like a reasonable check in register_console() that would reject bad console drivers.
Best Regards, Petr
| |