mount: optional arguments are expected to be NULL-ptr

[baloo]

Before this commit, the mount calls with None fstype or data, would show
up with empty strings:
mount("none", "/", 0x561882d38498, MS_REC|MS_PRIVATE, 0x561882d38498)

I believe they are expected to be passed as NULL instead like:
mount("none", "/", NULL, MS_REC|MS_PRIVATE, NULL) = 0

[baloo]

As suggested in #1213 I run a separate commit for rustfmt on src/mount.rs. Let me know if you want me to drop it.

I'm not too happy with the naming of things, but I couldn't come with a better one.

[asomers]

Why do you think that null pointers are preferable to empty strings? I can't find anything in the man pages for either FreeBSD or Linux that says one way or the other.

[baloo]

I'm trying to reimplement some behavior of unshare(1) (from https://mirrors.edge.kernel.org/pub/linux/utils/util-linux/) and well, it's calling with a NULL pointer, and I can't reproduce that behavior.

I have to agree that NULL behavior is not documented in the manpage.

[baloo]

All the tests in the linux kernel suggests it is supposed to be used with a NULL pointer as well. I can't find any good API documentation tough :/

[baloo]

a second version a bit less intrusive.

[baloo]

Turns out the root-cause of my problem was elsewhere. It works without this "fix" but I still think we should be using NULL pointers.

[asomers]

I'm not sure why we decided to treat empty arguments as empty strings instead of null pointers in the first place. That behavior was introduced by PR #231 by @kamalmarhubi . Perhaps he would know.

However, I realize now that we have a bigger problem that just mount(2). Since we implement NixPath for Option<NP: NixPath>, all of Nix's path-using functions accept None as an argument. For the vast majority, that doesn't make sense. access , for instance, returns ENOENT if you pass it an empty string. There's no reason why such functions should accept None.

A sensible if naive API for nix functions that need to take path arguments would be to take them as &CStr, or better yet (to save work for the callers) as something more like Into<CStr>. That's essentially what Nix had in its earliest days. But it's a problem if the user has a path that isn't nul-terminated. AFAICT, the NixPath trait was introduced precisely to deal with non-nul-terminated paths (see effb423). I think we should remove the NixPath implementation for Option types. Instead, each function that legitimately can accept a None argument should handle it itself. @kamalmarhubi thoughts?

[baloo]

I would think this is a reasonable argument. I can work on that.

As for the empty-string vs NULL-ptr, as far as I can tell, strace does not display empty-strings correctly. It just dumps the address, which makes it a bit more difficult to understand at first.

[asomers]

I got carried away; I'll have a patch soon. I'll try to do some more research RE empty strings vs null pointers. Have you checked what Linux's mount(8) does?

[asomers]

Actually, the answer is simple: the data argument it not a string, so it isn't valid to pass "" for that argument. Therefore, we must use a null pointer at least for that argument. Let's use a null pointer for all optional arguments, for consistency's sake.