Re: [PATCH -mm 1/9] unshare system call: system call handler function

[Jamie Lokier]

JANAK DESAI wrote:
> clone checks to
> makes sure that CLONE_NEWNS and CLONE_FS are not specified together, while
> unshare will force CLONE_FS if CLONE_NEWNS is spefcified.
[ ... ]
> since clone and unshare are doing opposite things, sharing code
> between them that checks constraints on the flags can get
> convoluted.

clone() and unshare() are not doing opposite things.  They do the same
thing, which is to unshare some properties while keeping others
shared, and the only differences are that clone() first creates a new
task, and the defaults for flags that aren't specified.

It is the polarity of _some_ flags which is opposite in your unshare()
API: In your API, CLONE_FS means "unshare fs", while with clone() it
means "share fs".  Same for other flags - except for CLONE_NEWNS,
where unshare() and clone() both take it to mean "unshare ns".

That's a bit of a confusing mixture of polarities, in my opinion.

I think it would be better if this:

	pid = clone(flags);

Could be always equivalent to this:

	pid = clone(CLONE_FLAGS_TO_SHARE_EVERYTHING)
	if (pid == 0)
		unshare(flags);

Or, if you don't like that, then this:

	pid = clone(CLONE_FLAGS_TO_SHARE_EVERYTHING)
	if (pid == 0)
		unshare(~flags);

However, if the API you've chosen for unshare() must be kept, then you
can still have the same checks in clone() and unshare().  Just XOR the
flags word with bits for polarities that are different between the two
calls, before doing the checks, and XOR again afterwards to include
any flags that were forced by the checks.

-- Jamie
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/