[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1157453094.3384.994.camel@quoit.chygwyn.com>
Date: Tue, 05 Sep 2006 11:44:54 +0100
From: Steven Whitehouse <swhiteho@...hat.com>
To: Jan Engelhardt <jengelh@...ux01.gwdg.de>
Cc: linux-kernel@...r.kernel.org,
Russell Cattelan <cattelan@...hat.com>,
David Teigland <teigland@...hat.com>,
Ingo Molnar <mingo@...e.hu>, hch@...radead.org
Subject: Re: [PATCH 06/16] GFS2: dentry, export, super and vm operations
Hi,
On Mon, 2006-09-04 at 18:35 +0200, Jan Engelhardt wrote:
> >> >+ switch (fh_type) {
> >> >+ case 10:
> >> >+ parent.no_formal_ino = ((uint64_t)be32_to_cpu(fh[4])) << 32;
> >> >+ parent.no_formal_ino |= be32_to_cpu(fh[5]);
> >> >+ parent.no_addr = ((uint64_t)be32_to_cpu(fh[6])) << 32;
> >> >+ parent.no_addr |= be32_to_cpu(fh[7]);
> >> >+ fh_obj.imode = be32_to_cpu(fh[8]);
> >> >+ case 4:
> >>
> >> What do these constants specify? Would not it be better to have a #define or
> >> enum{} for them somewhere?
> >
> >The sizes of the NFS file handles in units of sizeof(u32). I've added a
> >couple of #defines as requested.
>
> A #define/enum is only really useful if more than one place references it.
> If this is the only one, just add a comment.
>
There are several places as its used on both the fh encoding and fh
decoding routines.
> >> >+ if (IS_ERR(inode))
> >> >+ return ERR_PTR(PTR_ERR(inode));
> >>
> >> Just return inode.
> >
> >The function returns a dentry, so it would need to be casted and I
> >thought that would look "more odd" than this construction.
>
> Yes, it is very odd indeed that you return an inode as a dentry at all. How is
> the caller supposed to know whether it was an inode or dentry that was actually
> returned?
>
>
> Jan Engelhardt
This is dealing with the error case only. If the function being called
returns an error (signaled by IS_ERR(inode) - hence an invalid pointer
value) then we need to return that error to the calling routing. Since
this function in question returns a dentry, we convert the invalid
pointer value from the function returning an inode into an integer and
then covert the integer into an invalid pointer value again, but this
time its an invalid pointer to a dentry and hence the correct return
type for this function.
So the caller of this function will get a pointer to a dentry which will
either be a valid dentry pointer or an error value testable with
IS_ERR().
Steve.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists