kern_shm_open: push O_CLOEXEC into caller controller
ClosedPublic
Actions

Authored by kevans on Jul 30 2019, 7:50 PM.

Details

Reviewers

kib
markj

Commits

rS350464: kern_shm_open: push O_CLOEXEC into caller control

Summary

The motivation for this change is to allow wrappers around shm to be written that don't set CLOEXEC. kern_shm_open currently accepts O_CLOEXEC but sets it unconditionally. kern_shm_open is used by the shm_open(2) syscall, which is mandated by POSIX to set CLOEXEC, and CloudABI's sys_fd_create1(). Presumably O_CLOEXEC is intended in the latter caller, but it's unclear from the context.

sys_shm_open() now unconditionally sets O_CLOEXEC to meet POSIX requirements, and a comment has been dropped in to kern_fd_open() to explain the situation and add a pointer to where O_CLOEXEC setting is maintained for shm_open(2) correctness. CloudABI's sys_fd_create1() also unconditionally sets O_CLOEXEC to match previous behavior.

This also has the side-effect of making flags correctly reflect the O_CLOEXEC status on this fd for the rest of kern_shm_open(), but a glance-over leads me to believe that it didn't really matter.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kevans created this revision.Jul 30 2019, 7:50 PM

Herald added a subscriber: imp. · View Herald TranscriptJul 30 2019, 7:50 PM

Harbormaster completed remote builds in B25606: Diff 60306.Jul 30 2019, 7:50 PM

What about cloudabi_sys_fd_create1()? There is no reference to CLOEXEC at all in the cloudabi code, so it's not clear to me whether it cares about this particular case.

In D21119#458582, @markj wrote:

What about cloudabi_sys_fd_create1()? There is no reference to CLOEXEC at all in the cloudabi code, so it's not clear to me whether it cares about this particular case.

Hmm... not sure how I missed this one, I thought I grepped all of sys/. I guess it must set O_CLOEXEC to remain compatible, but maybe @ed can provide some insight as to whether that's correct going forward -- I kind of suspect it is.

kib added inline comments.Jul 30 2019, 9:45 PM

sys/kern/uipc_shm.c
737 ↗	(On Diff #60306)	sys_shm_open()

sys_shm_open -> sys_shm_open()
update previously missed cloudabi caller to also unconditionally set O_CLOEXEC to maintain previous behavior

Harbormaster completed remote builds in B25612: Diff 60314.Jul 31 2019, 3:09 AM

kib accepted this revision.Jul 31 2019, 5:16 AM

This revision is now accepted and ready to land.Jul 31 2019, 5:16 AM

markj accepted this revision.Jul 31 2019, 1:57 PM

Closed by commit rS350464: kern_shm_open: push O_CLOEXEC into caller control (authored by kevans). · Explain WhyJul 31 2019, 3:17 PM

This revision was automatically updated to reflect the committed changes.

kevans added a commit: rS350464: kern_shm_open: push O_CLOEXEC into caller control.

Revision Contents
Changeset List

Path

Size

head/

sys/

compat/

cloudabi/

cloudabi_fd.c

3 lines

kern/

uipc_shm.c

12 lines

Diff 60323

View Options

head/sys/compat/cloudabi/cloudabi_fd.c

Property	Old Value	New Value
fbsd:nokeywords	null	yes \ No newline at end of property
svn:eol-style	null	native \ No newline at end of property
svn:mime-type	null	text/plain \ No newline at end of property

				======================================================================
				You installed Sverchok: the Blender add-on for architects and
				designers.

				You need to enable it in Blender: click on the "File" menu, then on
				the "User Preferences" sub-menu, click on "Add-ons", type in "sverchok"
				into the search box in the top-left corner, and check the "Sverchok"
				check-box to the right.

				Please consult the user's manual:
				http://nikitron.cc.ua/sverch/html/main.html
				======================================================================