Re: "ls -l": Avoid unnecessary getxattr() overhead

[Jim Meyering]

Eric Blake wrote:
> On 02/17/2012 06:58 AM, Jim Meyering wrote:
>>>>   eval "LD_PRELOAD=$PWD/k.so ls --color=always -l . $fd>x" || exit 1
>>>>
>>>> Hmm... that means the case of no getxattr calls would still
>>>> require a little special handling to map "empty file" to "0".
>>>
>>> No.  Just add 'echo 0 >x' prior to the actual test, so that x is never
>>> empty.
>>
>> But the eval'd code will then truncate "x".
>
> Oh right.  But not the end of the world - in the eval, use $fd<>x to
> avoid the truncation.
>
>> On some systems there will be no intercepted *getxattr call,
>> and hence x will still be empty.
>>
>> Ahh... or maybe you meant to suggest that the eval'd code append to x?
>
> Alas, $fd>>x opens x with O_APPEND, so seeking to position 0 then
> calling write() will still append, which is not what we want.
>
> And hopefully no one is running this on the buggy version of dash, since
> it was only about 18 months ago where dash had a bug fix to quit using
> O_TRUNCATE for $fd<>x (bash and ksh have always followed that part of
> POSIX).
>
> Hmm, if we can't rely on $fd<>x, maybe we _can_ use $fd>>x coupled with
> fcntl() on fd to reset the O_APPEND before doing any write()s, but that
> starts to feel complicated.  I'd rather go with $fd<>x and tell users to
> upgrade to a fixed version of dash.

Thanks for the suggestions, but they go beyond my keep-it-simple
threshold :-)  It might seem odd to worry about simplicity at the
shell-redirect level in a test that uses LD_PRELOAD and gcc's destructor
attribute, but hey, I'll take it where I can.

I added a slightly different destructor than what I'd tried before,
and now it works:

Here's the stand-alone test:
----------------
fd=33
cat <<'EOF' > k.c
#include <errno.h>
#include <stdio.h>
static unsigned long int n_calls;
ssize_t getxattr (const char *path, const char *name, void *value, size_t size)
{ ++n_calls; errno = ENOTSUP; return -1; }
ssize_t lgetxattr(const char *path, const char *name, void *value, size_t size)
{ ++n_calls; errno = ENOTSUP; return -1; }
__attribute__((destructor)) void p()
{
  FILE *fp = fdopen (FD, "a"); if (!fp) return;
  fprintf (fp, "%lu\n", n_calls);
  fclose (fp);
}
EOF
gcc -DFD=$fd -Wall -fPIC -O2 -c k.c && ld -G k.o -o k.so &&
eval "LD_PRELOAD=./k.so ls -l --color=always . $fd>x" &&
cat x
----------------

And here's the patch I've just squashed into the test c-set.

diff --git a/tests/ls/getxattr-speedup b/tests/ls/getxattr-speedup
index eaa3342..a3d47c1 100755
--- a/tests/ls/getxattr-speedup
+++ b/tests/ls/getxattr-speedup
@@ -28,16 +28,18 @@ fd=33
 # determine the total number of calls.  FIXME: there must be a better way.
 cat > k.c <<'EOF' || framework_failure_
 #include <errno.h>
-#include <unistd.h>
-static void
-write_1 (void)
-{
-  write (FD, "x\n", 2);
-}
+#include <stdio.h>
+static unsigned long int n_calls;
 ssize_t getxattr (const char *path, const char *name, void *value, size_t size)
-{ write_1 (); errno = ENOTSUP; return -1; }
+{ ++n_calls; errno = ENOTSUP; return -1; }
 ssize_t lgetxattr(const char *path, const char *name, void *value, size_t size)
-{ write_1 (); errno = ENOTSUP; return -1; }
+{ ++n_calls; errno = ENOTSUP; return -1; }
+__attribute__((destructor)) void p()
+{
+  FILE *fp = fdopen (FD, "a"); if (!fp) return;
+  fprintf (fp, "%lu\n", n_calls);
+  fclose (fp);
+}
 EOF

 # Then compile/link it:
@@ -52,7 +54,7 @@ seq 100 | xargs touch || framework_failure_
 eval "LD_PRELOAD=./k.so ls --color=always -l . $fd>x" || fail=1

 # Ensure that there were no more than 3 *getxattr calls.
-n_calls=$(wc -l < x)
+n_calls=$(cat x)
 test "$n_calls" -le 3 || fail=1

 Exit $fail