Re: patch to fix io-seek on readonly stores

[Olivier Galibert <galibert@pobox.com>]

On Wed, Apr 26, 2000 at 11:26:37PM +0200, Marcus Brinkmann wrote:
> Anyway, here is a patch.

Well, I'm not (yet) a hurd hacker[1], but this patch strikes me as
fundamentally wrong (even if it fixes the problem for now).

The real problem is that io_seek uses a macro which is to be used only
when you want to _change_ an inode.  You need another macro:


/* This macro locks the node associated with PROTID, and then
   evaluates the expression OPERATION; then it unlocks everything, and
   returns the value `err' (which can be set by OPERATION if
   desired). */
#define ROACCESS_NODE_FIELD(PROTID, OPERATION)                              \
({                                                                          \
  error_t err = 0;                                                          \
  const struct node *np;                                                    \
                                                                            \
  if (!(PROTID))                                                            \
    return EOPNOTSUPP;                                                      \
                                                                            \
  np = (PROTID)->po->np;                                                    \
                                                                            \
  mutex_lock (&np->lock);                                                   \
  (OPERATION);                                                              \
  mutex_unlock (&np->lock);                                                 \
  return err;                                                               \
})

and change the macro used in io_seek.  And, of course, remove the
kludgy defines.

Also, the "const" helps at preventing you from shooting yourself in
the foot.

Some questions for the hurd hackers though:
- why should it be possible to pass NULL as the credential?  Should
  that be caught earlier, rather that duplicating the checking code in
  all of the macro instances?
- why isn't the node locking the responsability of the caller?
- why isn't diskfs_notice_filechange call the responsability of the
  caller?

  OG.

[1] The state of the X support and the impossibility to handle
decently-sized partitions is what has made it difficult for me to
really start working on the code