[LTP] [PATCH v8 0/4] Improve ioctl02.c
Petr Vorel
pvorel@suse.cz
Wed Oct 25 23:26:38 CEST 2023
Hi Marius,
> I implemented again the changes requested for the first and last
> patch.
+1
> That means I removed quite a few comments; I totally agree that
> this code was over-commented. I kept a few comments (mainly in the
> prepare function for the struct) because some struct members are
> over-abbreviated (e.g. `c_cc`) so the comments actually do help
> understanding the code. Some comments were also stating the
> intention of the code which also seemed not completely useless.
+1
> I dropped the intermediate patch to use just termios.
I don't know why, but that was for sure discussed in some previous versions.
Or do you want to get back to it after this effort gets merged?
> In the last patch I decided to use the double-assignment
> suggestion after all because the fields in termios are consistently
> wider than the fields in termio so when just swapping the
> assignment order this should be fine (there can never be a lossy
> conversion). I also decided to make the loop a macro as well
> because at this point we might as well go all-in with the macros.
We usually prefer functions, which are more readable. But I'll comment that more
at the 4th patch.
> Btw, you're sure you don't want to give C++ a try at some point :-)
> (Just mentioning this because I really would have liked using C++
> templates in this case and with C++ you can really pick what you
> like and keep everything else in the usual C-style.)
I don't think either C++ templates for LTP would be a good idea.
The code is very simple to include any templating system.
Also testing (g)libc headers should be IMHO done with plain C.
Kind regards,
Petr
> Marius Kittler (4):
> Refactor ioctl02.c to use the new test API
> Make checks for termio flags more strict
> Remove disabled code in ioctl02.c
> Extend ioctl02 to test termio and termios
> testcases/kernel/syscalls/ioctl/ioctl02.c | 551 +++++++---------------
> 1 file changed, 171 insertions(+), 380 deletions(-)
More information about the ltp
mailing list