Paths

Table of Contentst

-
sys/kern/
-
kern/
-
vfs_vnops.c

Differential D20783

style changes to FIOBMAP2
ClosedPublic
Actions

Authored by asomers on Jun 27 2019, 8:40 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Mon, Apr 14, 8:08 AM

	Unknown Object (File)
	Mon, Apr 14, 4:44 AM

	Unknown Object (File)
	Sun, Apr 13, 8:50 PM

	Unknown Object (File)
	Sun, Apr 13, 8:35 PM

	Unknown Object (File)
	Sun, Apr 13, 8:29 PM

	Unknown Object (File)
	Sun, Apr 6, 9:16 AM

	Unknown Object (File)
	Sat, Apr 5, 1:42 AM

	Unknown Object (File)
	Mar 13 2025, 2:26 AM

View All 43 Files

Subscribers

Details

Reviewers

Commits

rS349478: FIOBMAP2: inline vn_ioc_bmap2

Summary

style changes to FIOBMAP2

Reported-by: kib
MFC-With: 349231

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25098
Build 23800: arc lint + arc unit

Event Timeline

asomers created this revision.Jun 27 2019, 8:40 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 27 2019, 8:40 PM

Harbormaster completed remote builds in B25092: Diff 59115.Jun 27 2019, 8:40 PM

kib added inline comments.Jun 27 2019, 9:06 PM

sys/kern/vfs_vnops.c
1459–1460	Do you need such trivial helper at all ? IMO you can directly call VOP_BMAP() in vn_ioctl().
1460	`return (VOP_BMAP(...`
1492	We typically indent as if MAC == 1 always (look at other similar places).

asomers added inline comments.Jun 27 2019, 10:07 PM

sys/kern/vfs_vnops.c
1459–1460	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`.

kib added inline comments.Jun 27 2019, 10:41 PM

sys/kern/vfs_vnops.c
1459–1460	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.

inline vn_ioc_bmap2

Harbormaster completed remote builds in B25098: Diff 59133.Jun 27 2019, 11:07 PM

kib accepted this revision.Jun 27 2019, 11:31 PM

This revision is now accepted and ready to land.Jun 27 2019, 11:31 PM

Closed by commit rS349478: FIOBMAP2: inline vn_ioc_bmap2 (authored by asomers). · Explain WhyJun 27 2019, 11:39 PM

This revision was automatically updated to reflect the committed changes.

asomers added a commit: rS349478: FIOBMAP2: inline vn_ioc_bmap2.

asomers mentioned this in rS350956: MFC r349231, r349233, r349280, r349478.Aug 12 2019, 8:21 PM

Revision Contents
Changeset List

Path

Size

sys/

kern/

33 lines

Diff 59133

sys/kern/vfs_vnops.c

	Show First 20 Lines • Show All 97 Lines • ▼ Show 20 Lines
	* seem to work. Use old-schoold-ish CTASSERT from before commit			* seem to work. Use old-schoold-ish CTASSERT from before commit
	* a3085588a88fa58eb5b1eaae471999e1995a29cf but also make sure we do not			* a3085588a88fa58eb5b1eaae471999e1995a29cf but also make sure we do not
	* end up with an unused typedef or variable. The compiler should optimise			* end up with an unused typedef or variable. The compiler should optimise
	* it away entirely.			* it away entirely.
	*/			*/
	#define _O_CTASSERT(x) _O__CTASSERT(x, __LINE__)			#define _O_CTASSERT(x) _O__CTASSERT(x, __LINE__)
	#define _O__CTASSERT(x, y) _O___CTASSERT(x, y)			#define _O__CTASSERT(x, y) _O___CTASSERT(x, y)
	#define _O___CTASSERT(x, y) while (0) { \			#define _O___CTASSERT(x, y) while (0) { \
	typedef char __assert_line_ ## y[(x) ? 1 : -1]; \			typedef char __unused __assert_line_ ## y[(x) ? 1 : -1]; \
	__assert_line_ ## y _x; \			__assert_line_ ## y _x; \
	_x[0] = '\0'; \			_x[0] = '\0'; \
	}			}

	#define BUILD_BUG() do { CTASSERT(0); } while (0)			#define BUILD_BUG() do { CTASSERT(0); } while (0)
	#define BUILD_BUG_ON(x) do { _O_CTASSERT(!(x)) } while (0)			#define BUILD_BUG_ON(x) do { _O_CTASSERT(!(x)) } while (0)
	#define BUILD_BUG_ON_MSG(x, msg) BUILD_BUG_ON(x)			#define BUILD_BUG_ON_MSG(x, msg) BUILD_BUG_ON(x)
	#define BUILD_BUG_ON_NOT_POWER_OF_2(x) BUILD_BUG_ON(!powerof2(x))			#define BUILD_BUG_ON_NOT_POWER_OF_2(x) BUILD_BUG_ON(!powerof2(x))
	▲ Show 20 Lines • Show All 650 Lines • Show Last 20 Lines