On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote: > There are several console drivers which have same checks w.r.t. printable > characters. The check is moved to new isconsole() macro and re-used in > the console drivers.
Something named isconsole() imo can't be expected to do what is checked for ... > --- a/xen/arch/arm/vuart.c > +++ b/xen/arch/arm/vuart.c > @@ -79,8 +79,7 @@ static void vuart_print_char(struct vcpu *v, char c) > struct domain *d = v->domain; > struct vuart *uart = &d->arch.vuart; > > - /* Accept only printable characters, newline, and horizontal tab. */ > - if ( !isprint(c) && (c != '\n') && (c != '\t') ) > + if ( !isconsole(c) ) > return ; ... e.g. here. If we really want such a further abstraction (of which I'm unconvinced), then maybe isprintable() or (getting ling-ish) is_console_printable(). > --- 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. > */ How's this change related to the purpose of the patch? > @@ -30,6 +32,7 @@ extern const unsigned char _ctype[]; > #define isspace(c) ((__ismask(c)&(_S)) != 0) > #define isupper(c) ((__ismask(c)&(_U)) != 0) > #define isxdigit(c) ((__ismask(c)&(_D|_X)) != 0) > +#define isconsole(c) (isprint(c) || (c) == '\n' || (c) == '\t') In a pretty general purpose macro like this one I'm afraid I view it as risky to evaluate the parameter more than once. Jan
