Page MenuHomeFreeBSD

style changes to FIOBMAP2
ClosedPublic

Authored by asomers on Jun 27 2019, 8:40 PM.
Tags
None
Referenced Files
F103388971: D20783.id59115.diff
Sun, Nov 24, 9:32 AM
Unknown Object (File)
Thu, Nov 21, 9:41 PM
Unknown Object (File)
Thu, Nov 7, 2:13 AM
Unknown Object (File)
Sep 24 2024, 7:16 AM
Unknown Object (File)
Sep 17 2024, 9:05 PM
Unknown Object (File)
Sep 17 2024, 4:36 AM
Unknown Object (File)
Sep 16 2024, 12:25 AM
Unknown Object (File)
Sep 4 2024, 9:38 PM
Subscribers

Details

Summary

style changes to FIOBMAP2

Reported-by: kib
MFC-With: 349231

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/kern/vfs_vnops.c
1465 ↗(On Diff #59115)

return (VOP_BMAP(...

1466 ↗(On Diff #59115)

Do you need such trivial helper at all ? IMO you can directly call VOP_BMAP() in vn_ioctl().

1500 ↗(On Diff #59115)

We typically indent as if MAC == 1 always (look at other similar places).

sys/kern/vfs_vnops.c
1466 ↗(On Diff #59115)

I did it to avoid bloating vn_ioctl, and to separate the concerns of parsing com from implementing an ioctl. Would you be happier with the original approach if I used some other naming convention for vn_ioc_bmap2? I see that there are other functions in this file that take a struct file* argument too, like vn_read and vn_write.

sys/kern/vfs_vnops.c
1466 ↗(On Diff #59115)

FIONREAD case looks fine and not bloated. In the vn_ioc_bmap2() interface, I do not like that the file * is passed where function really operates on vnode, and that the ABI structure is passed as an argument to kernel helper. This does not depend on name.

FWIW, vn_read, or better, vn_write, operate on a lot of state in file instead of only vnode.

This revision is now accepted and ready to land.Jun 27 2019, 11:31 PM
This revision was automatically updated to reflect the committed changes.