Re: [PATCH] openat: reduce syscalls in first probe of /proc

[Jim Meyering]

Eric Blake wrote:
> open/access/close is cheaper than open/stat/stat/close.
>
> * lib/openat-proc.c (openat_proc_name): Simplify.
> * modules/openat (Depends-on): Drop same-inode.
> Reported by Bastien ROUCARIES.
...
> diff --git a/lib/openat-proc.c b/lib/openat-proc.c
...
> @@ -80,15 +79,9 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, 
> char const *file)
>          proc_status = -1;
>        else
>          {
> -          struct stat proc_self_fd_dotdot_st;
> -          struct stat proc_self_st;
> -          char dotdot_buf[PROC_SELF_FD_NAME_SIZE_BOUND (sizeof ".." - 1)];
> -          sprintf (dotdot_buf, PROC_SELF_FD_FORMAT, proc_self_fd, "..");
> -          proc_status =
> -            ((stat (dotdot_buf, &proc_self_fd_dotdot_st) == 0
> -              && stat ("/proc/self", &proc_self_st) == 0
> -              && SAME_INODE (proc_self_fd_dotdot_st, proc_self_st))
> -             ? 1 : -1);
> +          char dotdot_buf[PROC_SELF_FD_NAME_SIZE_BOUND (sizeof "../fd" - 1)];
> +          sprintf (dotdot_buf, PROC_SELF_FD_FORMAT, proc_self_fd, "../fd");
> +          proc_status = access (dotdot_buf, F_OK) ? -1 : 1;
>            close (proc_self_fd);

Thanks.  That looks fine, but I had to study it for long enough
that an additional comment would have been welcome.  I added this:


>From 5f1e41e3755fc46e97ca3ada87779d60751417d2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 10 May 2011 09:46:59 +0200
Subject: [PATCH] openat: add comments

* lib/openat-proc.c (openat_proc_name): Add comments,
mostly from Eric Blake.
---
 ChangeLog         |    6 ++++++
 lib/openat-proc.c |    4 ++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 36fb1fe..5a89d18 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-05-10  Jim Meyering  <address@hidden>
+
+       openat: add comments
+       * lib/openat-proc.c (openat_proc_name): Add comments,
+       mostly from Eric Blake.
+
 2011-05-09  Eric Blake  <address@hidden>

        openat: reduce syscalls in first probe of /proc
diff --git a/lib/openat-proc.c b/lib/openat-proc.c
index 5711896..c326ffc 100644
--- a/lib/openat-proc.c
+++ b/lib/openat-proc.c
@@ -80,6 +80,10 @@ openat_proc_name (char buf[OPENAT_BUFFER_SIZE], int fd, char 
const *file)
         proc_status = -1;
       else
         {
+          /* Detect whether /proc/self/fd/../fd exists.  On Linux, that name
+             resolves to /proc/self/fd, which was opened above.  However, on
+             Solaris, it may resolve to /proc/self/fd/fd, which cannot exist,
+             since all names in /proc/self/fd are numeric.  */
           char dotdot_buf[PROC_SELF_FD_NAME_SIZE_BOUND (sizeof "../fd" - 1)];
           sprintf (dotdot_buf, PROC_SELF_FD_FORMAT, proc_self_fd, "../fd");
           proc_status = access (dotdot_buf, F_OK) ? -1 : 1;
--
1.7.5.1.354.g761178