2014-07-05 20:51:22

by Steven Stewart-Gallus

[permalink] [raw]
Subject: [PATCH 1/1] include/uapi: Define AT_FDNODIR constant as future guarantee

From: Steven Stewart-Gallus <[email protected]

This constant means that in the far future it might be possible to
define other AT_FD* constants.

Signed-off-by: Steven Stewart-Gallus <[email protected]>
---

This is my first kernel patch but this is really trivial so I hope I'm doing
this right.

diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 074b886..92223f0 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -38,9 +38,15 @@
#define DN_ATTRIB 0x00000020 /* File changed attibutes */
#define DN_MULTISHOT 0x80000000 /* Don't remove notifier */

+#define AT_FDNODDIR -1 /* Special value used to indicate
+ openat should not use any directory.
+ Currently, other values work for this
+ but in the future that might
+ change. */
#define AT_FDCWD -100 /* Special value used to indicate
- openat should use the current
- working directory. */
+ openat should use the current
+ working directory. */
+
#define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic links. */
#define AT_REMOVEDIR 0x200 /* Remove directory instead of
unlinking file. */


2014-07-08 12:12:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/1] include/uapi: Define AT_FDNODIR constant as future guarantee

On Sat, Jul 05, 2014 at 07:51:19PM +0000, Steven Stewart-Gallus wrote:
> From: Steven Stewart-Gallus <[email protected]
>
> This constant means that in the far future it might be possible to
> define other AT_FD* constants.
>
> Signed-off-by: Steven Stewart-Gallus <[email protected]>
> ---
>
> This is my first kernel patch but this is really trivial so I hope I'm doing
> this right.

Almost. For one you really should explain what the flag does,
especially if it's not actually use.

It took me a while that you're trying to document the way Linux
currently behaves for the cases where the dfd argument to the various
*at is neither a valid file descriptor nor the magic AT_FDCWD constant.

Posix says that openat with an absolute path is equivalent to open, so
the only strictly allowed other flags would those that allow some form
of relative path, but treat it differently than the currently possible
options. I can't really think of a good way to do that, although we
recently had a case where a different AT_* value would be useful, even
if not fully conforming to that Posix wording.

Your explanation of what this constant does should also go into the man
page, so in addition to the kernel patch you should also propose a patch
to the man page documenting it.

Also I don't think AT_FDNODIR is a good choice of name, AT_ABSOLUTE
would be a lot more descriptive.

2014-07-09 23:43:27

by Steven Stewart-Gallus

[permalink] [raw]
Subject: Re: [PATCH 1/1] include/uapi: Define AT_FDNODIR constant as future guarantee

Thank you for the criticisms Christopher Hellwig.

I will edit the commit message to be better and comments in the code
to be clearer. I will also document the constant in the manpages. I
think I will call the constant AT_FDABSOLUTE though because I want it
to be prepended with FD the same way AT_FDCWD is.

Thank you,
Steven Stewart-Gallus