Re: [LTP] [PATCH v7 5/5] Extend ioctl02 to test termio and termios

[Marius Kittler <mkittler@suse.de>]

Am Dienstag, 24. Oktober 2023, 17:54:27 CEST schrieb Martin Doucha:

> You don't need to duplicate the whole prepare_termio() function. You can
> either initialize both structures at once:
> termio.c_line = termios.c_line = 0;
> termio.c_cflag = termios.c_cflag = ...;

That's not possible with all the assignments because the variables have 
different types (e.g. unsigned short vs. unsigned int). These different types 
are the reason why there are different structs in the first place. (Of course it 
would technically be possible but introducing implicit conversions and relying 
on these conversions not being too lossy is a bad idea in my opinion.)

> of #define an init macro:
> #define TERM_INIT_VALUES { .c_line = 0, .c_cflag = ... }
> termio = (struct termio)TERM_INIT_VALUES;
> termios = (struct termios)TERM_INIT_VALUES;
> 

That is rather ugly but likely the way to go then. And I assume I'd call this 
"combined" prepare function during the setup then.

> Some code duplication is unavoidable here but the value validation can
> be simplified:
> 
> 1) Add another pair of termio+termios variables which will be
> initialized in setup() and passed only to SAFE_IOCTL(TCSETA/TCSETS).
> 
> 2) memset() the existing termio+termios variables before passing them to
> SAFE_IOCTL(TCGETA/TCGETS). As is, the test can pass even if
> TCGETA/TCGETS does nothing because they store data into a structure that
> already contains the exact same values.
> 
> 3) Define a macro to compare the "set" and "get" variables:
> #define CMP_ATTR(tcexp, tcval, attr) \
> 	if ((tcval).attr != (tcexp).attr) { \
> 		tst_res(TINFO, #attr " has incorrect value %o", \
> 			 (tcval).attr); \
> 		flag++; \
> 	}
> 
> Then all attribtute checks except the loop can be reduced to one line.
> 

I guess that is the least ugly way to use macros, indeed. (Supposedly better 
than just wrapping the whole function body into a macro.)





-- 
Mailing list info: https://lists.linux.it/listinfo/ltp