search

linux-test-project / ltp

Linux Test Project (mailing list: https://lists.linux.it/listinfo/ltp)
https://linux-test-project.readthedocs.io/
GNU General Public License v2.0
2.33k stars 1.02k forks source link

ioctl: Test also struct termios in ioctl tests #637

Open pevik opened 4 years ago

pevik commented 4 years ago

ATM we're testing just legacy struct termio in ioctl01.c and ioctl02.c.

https://lists.linux.it/pipermail/ltp/2020-January/015236.html https://patchwork.ozlabs.org/patch/1230659/

coolgw commented 1 year ago

https://patchwork.ozlabs.org/project/ltp/list/?series=339112

Martchus commented 1 year ago

The patchwork link shows no results but it looks like this has already been implemented via https://github.com/linux-test-project/ltp/commit/813457d01c2b3db6e1f5f1fa4291b932e132bb30. So this issue can probably be closed. (I'm currently looking for issues tagged with "easyhack" to get started and stumbled across this one.)

metan-ucw commented 1 year ago

Looking at the patch https://github.com/linux-test-project/ltp/commit/813457d01c2b3db6e1f5f1fa4291b932e132bb30 it seems to be wrong though. The TCGETA for termio is called TCGETS for termios, we need separate lines with different ioctl cmds for different structures. And looking at man ioctl_tty there is termios2 too, with TCGETS2, hence we need to cover that as well.

Martchus commented 1 year ago

Ok, then maybe a ticket for me to pick-up after all. I guess I'll have a look whether I can make sense of what you're saying.

Martchus commented 1 year ago

I think I understood it. I'll send a patch for the first part (TCGETA vs. TCGETS). It is a while ago since I have sent a patch to a mailing list so likely this will be more complicated than the change itself.

metan-ucw commented 1 year ago

Also looking at the ioctl02, that one has to be cleaned up, converted to the new test API and needs termios support to be added. Generally the testing for termios seems to be sparse and we probably miss quite a bit of possible coverage.

Martchus commented 1 year ago

Yes. I'll work on ioctl02 tomorrow. Cleaning it up should be a good way of getting familiar with the code base.

Martchus commented 1 year ago

Note that I've sent patches to the LTP list for ioctl01.c and the refactoring part of ioctl02.c. I'm currently waiting for feedback.

http://patchwork.ozlabs.org/project/ltp/patch/20230905093019.13881-2-mkittler@suse.de/ http://patchwork.ozlabs.org/project/ltp/patch/20230905093019.13881-3-mkittler@suse.de/ http://patchwork.ozlabs.org/project/ltp/patch/20230905115608.31192-2-mkittler@suse.de/ http://patchwork.ozlabs.org/project/ltp/patch/20230905115608.31192-3-mkittler@suse.de/

Martchus commented 1 year ago

The improvements for ioctl01.c have been merged so termios is tested (using the correct request). So I guess this issue is technically resolved although I'm still waiting for feedback for the refactoring of ioctl02.c.

pevik commented 1 year ago

The patchwork link shows no results but it looks like this has already been implemented via 813457d.

FYI: When you click to icon next to Action Required to disable this filter, you see the patch with status Changes Requested, which is disabled by default view: https://patchwork.ozlabs.org/project/ltp/list/?series=339112&state=*