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

[Denis Mukhin <dmkhn@xxxxxxxxx>]

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.

> 
> > @@ -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