Re: [Gnu-arch-users] [HEY TOM!] unhelpful error for unreadable directories

[Tom Lord]

    > From: Aaron Bentley <address@hidden>

    > This came up again in IRC last night.  An unreadable directory caused 
    > inventory traversal to halt with:

    > PANIC: i/o error in arch_inventory_traversal/is_nested_tree

    > No filename, no perror-style message.  We ended up using strace to find 
    > the problem.

    > There's another error in traversal that does work this way:
    > i/o error during inventory traversal (%s) for %s\n", errno_to_string 
    > (errn), file

    > I'd like to do that here, but, the filename in question is $foo/{arch}, 
    > where $foo may or may not be a directory, and {arch} usually doesn't 
exist.

    > Any suggestions for how to handle this?

You're talking about this code, right?

  ctl_file = file_name_in_vicinity (0, path, "{arch}");

  if (0 > vu_lstat (&errn, ctl_file, &stat_buf))
    {
      if (errn == ENOENT)
        {
          lim_free (0, ctl_file);
          return 0;
        }
      else
        {
          panic ("i/o error in arch_inventory_traversal/is_nested_tree");
          return 0;             /* notreached */
        }
    }

And you're wondering whether to just report an error with `ctl_file'
or whether to attempt some further diagnosis and maybe report an error
with `path' instead?

I'd suggest writing `safe_file_or_symlink_exists' (and, for symmetry,
`safe_file_exists') to go in new files in src/hackerlab/fs.   I could
have sworn I already had those but apparently not.    Replace the
entire conditional with:

        answer = safe_file_or_symlink_exists (ctl_file);
        lim_free (0, ctl_file);
        return answer;

I _think_ this is an uncommon-enough, weird-enough situation that we
can live with a "PANIC" although, sure, a more informative panic
message would be nice.

What should safe_file_or_symlink_exists print?

As a general rule (with rare exceptions) you _must_ report the
important arguments and system call used (or a reasonable subset of
that information) when you get an expected error from a filesystem
call.

If, instead, you issue additional system calls to try to figure out
why specifically you got an error, your code will be incorrect -- too
clever for its own good.  For example, it will be fooled by transient
errors, secondary errors, and intervening changes to the filesystem.

You could print:

        "i/o error accessing %s (%s)\n", ctl_file, errno_to_string...


or even:

        "i/o error (in lstat) accessing %s (%s)\n", ctl_file, errno_to_string...



It would be too excessive (I wouldn't accept it) but you could
correctly print (after performing suitable tests):

        "i/o error accessing %s (%s)\n", ctl_file, errno_to_string...
        "perhaps because %s is not readable\n", path

But it is impossible to write usefully correct code that would print
only:

        "%s is not readable\n", path

(The message printed may be correct but it may not be what the actual
problem was.)

-t