Re: [PATCH 06/16] GFS2: dentry, export, super and vm operations

[Steven Whitehouse <swhiteho@redhat.com>]

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/