Re: [PATCH v3 01/24] xen/ctype: introduce is_console_printable()

[Denis Mukhin <dmkhn@xxxxxxxxx>]

On Thursday, January 23rd, 2025 at 1:11 PM, Denis Mukhin <dmkhn@xxxxxxxxx> 
wrote:

> 
> 
> On Tuesday, January 21st, 2025 at 11:19 PM, Jan Beulich jbeulich@xxxxxxxx 
> wrote:
> 
> > On 04.01.2025 02:58, Denis Mukhin via B4 Relay wrote:
> > 
> > > --- a/xen/include/xen/ctype.h
> > > +++ b/xen/include/xen/ctype.h
> > > @@ -4,6 +4,8 @@
> > > /*
> > > * NOTE! This ctype does not handle EOF like the standard C
> > > * library is required to.
> > > + *
> > > + * See Rule 21.13 in docs/misra/rules.rst.
> > > */
> > 
> > As previously indicated, I object to such comments. I think I said so 
> > before:
> > All Misra rules are relevant everywhere anyway.
> 
> 
> Correct, I received that feedback in v2 comments, but after I posted v3
> on the mailing list.
> 
> That will be addressed in the next iteration.

Moved to 
  https://lore.kernel.org/xen-devel/20250207005532.345746-1-dmkhn@xxxxxxxxx

> 
> > > @@ -51,4 +53,9 @@ static inline unsigned char __toupper(unsigned char c)
> > > #define tolower(c) ((char)__tolower(c))
> > > #define toupper(c) ((char)__toupper(c))
> > > 
> > > +static inline unsigned is_console_printable(unsigned char c)
> > > +{
> > > + return isprint(c) || c == '\n' || c == '\t';
> > > +}
> > 
> > Why a return type of unsigned (and then not even "unsigned int")? I can't
> > spot anything in the file which would serve as a reference for this, and
> > by its nature the function clearly wants to return bool.
> > 
> > I further question the placement of this function in ctype.h: Only console
> > related code cares about this function, so exposure is far too wide this
> > way.
> 
> 
> I will move that to console.h in v4.
> Thanks.
> 
> > Jan