* netfs.c (netfs_attempt_sync): Move err inside the loop. Declare
final_err. On an error, store the error and go to the next item.
Check for the directory port being NULL.
---
Hello,
On Fri, Aug 14, 2009 at 08:26:18PM +0200, Thomas Schwinge wrote:
> On Fri, Aug 14, 2009 at 07:41:08PM +0300, Sergiu Ivanov wrote:
> > On Fri, Aug 14, 2009 at 05:18:59PM +0200, Thomas Schwinge wrote:
> > > Why did you send it in a separate message by the way? Anyways, I'll post
> > > my few comments in here.
> >
> > I just didn't think about sending it in one message. (I hope this one
> > is formatted properly).
>
> Jup, arrived just fine, and I was able to import it into my repository
> with a simple ``git am < [file]''. Doing it this way, the advantage is
> that all what belongs together can easily be viewed together (that is,
> the discussion and the patch), without having to juggle with two emails
> in parallel.
Great :-) I'll try to remember to always do it in this way.
> > node_ulfs_iterate_unlocked (np)
> > {
> > + error_t err;
> > +
> > /* Get the information about the current filesystem. */
> > err = ulfs_get_num (i, &ulfs);
> > if (err)
> > - break;
> > + {
> > + final_err = err;
> > + continue;
> > + }
>
> Looking at ulfs_get_num's implementation I wonder whether we should
> actually report its failing back to the invoker of netfs_attempt_sync?
> Wouldn't ulfs_get_num failing be a sign of corrupted internal state? So,
> would either a silent continue or even a assert (err == 0) be more
> appropriate here?
You are right -- if a node contains more node_ulfs entries than there
are registered in ulfs_chain, then something has gone seriously
corrupted. However, I have a question, which is related to
consistency (again *sigh*): ulfs_get_num is invoked in two places
only: in netfs_attempt_sync and in node_init_root (node.c:533 in my
code version). In node_init_root the return value of ulfs_get_num is
checked in an if statement. Is it alright to check this value via an
assert in netfs_attempt_sync? Or should I change the handling of the
return value in node_init_root instead?
Regards,
scolobb
---
netfs.c | 17 +++++++++++------
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/netfs.c b/netfs.c
index b3174fb..e2a6f65 100644
--- a/netfs.c
+++ b/netfs.c
@@ -283,7 +283,8 @@ error_t
netfs_attempt_sync (struct iouser *cred, struct node *np,
int wait)
{
- error_t err = 0;
+ /* The error we are going to report back (last failure wins). */
+ error_t final_err = 0;
/* The index of the currently analyzed filesystem. */
int i = 0;
@@ -300,23 +301,27 @@ netfs_attempt_sync (struct iouser *cred, struct node *np,
place, because now iterating over both lists looks ugly. */
node_ulfs_iterate_unlocked (np)
{
+ error_t err;
+
/* Get the information about the current filesystem. */
err = ulfs_get_num (i, &ulfs);
- if (err)
- break;
+ assert (err == 0);
- if (ulfs->flags & FLAG_ULFS_WRITABLE)
+ /* Since `np` may not necessarily be present in every underlying
+ directory, having a null port is perfectly valid. */
+ if ((node_ulfs->port != MACH_PORT_NULL)
+ && (ulfs->flags & FLAG_ULFS_WRITABLE))
{
err = file_sync (node_ulfs->port, wait, 0);
if (err)
- break;
+ final_err = err;
}
++i;
}
mutex_unlock (&ulfs_lock);
- return err;
+ return final_err;
}
/* This should sync the entire remote filesystem. If WAIT is set,
--
1.6.3.3
