In fs/nfsd/nfsfh.c:fh_verify(), the following block of code is
responsible for type checking:
/* Type check. The correct error return for type mismatches
* does not seem to be generally agreed upon. SunOS seems to
* use EISDIR if file isn't S_IFREG; a comment in the NFSv3
* spec says this is incorrect (implementation notes for the
* write call).
*/
/* Type can be negative when creating hardlinks - not to a dir */
if (type > 0 && (inode->i_mode & S_IFMT) != type) {
error = (type == S_IFDIR)? nfserr_notdir : nfserr_isdir;
goto out;
}
if (type < 0 && (inode->i_mode & S_IFMT) == type) {
error = (type == -S_IFDIR)? nfserr_notdir : nfserr_isdir;
goto out;
}
I think that the assignment in the last block is backwards and should be
error = (type == -S_IFDIR)? nfserr_isdir : nfserr_notdir;
Also, how about changing the assignment in the first block to
if (type == S_IFDIR)
error = nfserr_notdir;
else if ((inode->i_mode & S_IFMT) == S_IFDIR)
error = nfserr_isdir;
else
error = nfserr_inval;
goto out;
to be consistent with the comment?
Cheers,
Kendrick
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Saturday July 27, [email protected] wrote:
>
> In fs/nfsd/nfsfh.c:fh_verify(), the following block of code is
> responsible for type checking:
>
> /* Type check. The correct error return for type mismatches
> * does not seem to be generally agreed upon. SunOS seems to
> * use EISDIR if file isn't S_IFREG; a comment in the NFSv3
> * spec says this is incorrect (implementation notes for the
> * write call).
> */
>
> /* Type can be negative when creating hardlinks - not to a dir */
> if (type > 0 && (inode->i_mode & S_IFMT) != type) {
> error = (type == S_IFDIR)? nfserr_notdir : nfserr_isdir;
> goto out;
> }
> if (type < 0 && (inode->i_mode & S_IFMT) == type) {
> error = (type == -S_IFDIR)? nfserr_notdir : nfserr_isdir;
> goto out;
> }
>
> I think that the assignment in the last block is backwards and should be
>
> error = (type == -S_IFDIR)? nfserr_isdir : nfserr_notdir;
Looks right... but we should also change
if (type < 0 && (inode->i_mode & S_IFMT) == type) {
to
if (type < 0 && (inode->i_mode & S_IFMT) == -type) {
or we will never benefit from the change !!!
>
> Also, how about changing the assignment in the first block to
>
> if (type == S_IFDIR)
> error = nfserr_notdir;
> else if ((inode->i_mode & S_IFMT) == S_IFDIR)
> error = nfserr_isdir;
> else
> error = nfserr_inval;
> goto out;
>
> to be consistent with the comment?
Sounds fair to me.
Would you like to convert this into a patch and send it to Linus with
my blessing ?
I probably wouldn't bother Marcelo with it until 2.4.19 is out.
Thanks,
NeilBrown
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
On Sun, 28 Jul 2002, Neil Brown wrote:
> On Saturday July 27, [email protected] wrote:
> >
> > In fs/nfsd/nfsfh.c:fh_verify(), the following block of code is
> > responsible for type checking:
> >
> > /* Type check. The correct error return for type mismatches
> > * does not seem to be generally agreed upon. SunOS seems to
> > * use EISDIR if file isn't S_IFREG; a comment in the NFSv3
> > * spec says this is incorrect (implementation notes for the
> > * write call).
> > */
> >
> > /* Type can be negative when creating hardlinks - not to a dir */
> > if (type > 0 && (inode->i_mode & S_IFMT) != type) {
> > error = (type == S_IFDIR)? nfserr_notdir : nfserr_isdir;
> > goto out;
> > }
> > if (type < 0 && (inode->i_mode & S_IFMT) == type) {
> > error = (type == -S_IFDIR)? nfserr_notdir : nfserr_isdir;
> > goto out;
> > }
> >
> > I think that the assignment in the last block is backwards and should be
> >
> > error = (type == -S_IFDIR)? nfserr_isdir : nfserr_notdir;
>
> Looks right... but we should also change
> if (type < 0 && (inode->i_mode & S_IFMT) == type) {
> to
> if (type < 0 && (inode->i_mode & S_IFMT) == -type) {
>
> or we will never benefit from the change !!!
Sorry! This was a typo in my message: it should have been -type
to begin with...
> > Also, how about changing the assignment in the first block to
> >
> > if (type == S_IFDIR)
> > error = nfserr_notdir;
> > else if ((inode->i_mode & S_IFMT) == S_IFDIR)
> > error = nfserr_isdir;
> > else
> > error = nfserr_inval;
> > goto out;
> >
> > to be consistent with the comment?
>
> Sounds fair to me.
> Would you like to convert this into a patch and send it to Linus with
> my blessing ?
> I probably wouldn't bother Marcelo with it until 2.4.19 is out.
OK, I will send a patch in the next day or two.
Cheers,
Kendrick
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs