From mboxrd@z Thu Jan 1 00:00:00 1970 From: Namjae Jeon Subject: Re: [PATCH v5 5/8] fat: restructure export_operations Date: Wed, 5 Dec 2012 14:58:39 +0900 Message-ID: References: <1353504277-5947-1-git-send-email-linkinjeon@gmail.com> <87hao35uqz.fsf@devron.myhome.or.jp> <87obia17zr.fsf@devron.myhome.or.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Namjae Jeon , Ravishankar N , Amit Sahrawat To: OGAWA Hirofumi Return-path: Received: from mail-qa0-f46.google.com ([209.85.216.46]:57871 "EHLO mail-qa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751809Ab2LEF6k convert rfc822-to-8bit (ORCPT ); Wed, 5 Dec 2012 00:58:40 -0500 In-Reply-To: <87obia17zr.fsf@devron.myhome.or.jp> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 2012/12/4, OGAWA Hirofumi : > Namjae Jeon writes: > >>>> +static struct dentry *fat_fh_to_dentry_nostale(struct super_block= *sb, >>>> + struct fid *fh, int fh_len, >>>> + int fh_type) >>>> +{ >>>> + struct inode *inode =3D NULL; >>>> + struct fat_fid *fid =3D (struct fat_fid *)fh; >>>> + loff_t i_pos; >>>> + >>>> + switch (fh_type) { >>>> + case FILEID_FAT_WITHOUT_PARENT: >>>> + if (fh_len < FAT_FID_SIZE_WITHOUT_PARENT) >>>> + return NULL; >>>> + case FILEID_FAT_WITH_PARENT: >>>> + if ((fh_len < FAT_FID_SIZE_WITH_PARENT) && >>>> + (fh_type =3D=3D FILEID_FAT_WITH_PARENT)) >>>> + return NULL; >>> >>> Do we have to care (FILEID_FAT_WITH_PARENT and fh_len < 5) here? >>> >>> if (fh_len < 2) >>> return NULL; >>> >>> switch (fh_type) { >>> case FILEID_INO32_GEN: >>> case FILEID_INO32_GEN_PARENT: >>> inode =3D get_inode(sb, fid->i32.ino, fid->i32.gen); >>> break; >>> } >>> >>> return d_obtain_alias(inode); >>> >>> generic_fh_to_dentry() is above. I wonder why we have to care >>> fat_fid->parent* here. >> Let me think, if =E2=80=98subtree=E2=80=99 checking is enabled then = we should check >> the length condition over here also? Please share if there are any >> other comments also. > > I'm not sure what did you mean. Where is "subtree" check you are > talking? This is fh_to_dentry(), so we don't use parent at all, so > length =3D=3D 3 is enough? With fileID type=3D"FILEID_FAT_WITHOUT_PARENT", fhlen should be '3' With fileId type=3D"FILEID_FAT_WITH_PARENT", fhlen should be '5' While encoding, WITH_PARENT is selected when subtree check is enabled on the NFS Server. So, when decoding request is arrived- fileid type will be among the '2= ' cases: Now, in case of fh_to_dentry() - when we consider, that the reqquest for fileid type WITH_PARENT then i think the conditions in fh_to_dentry should be: if((fh_type =3D=3D FILEID_FAT_WITH_PARENT) && fh_len !=3D 5) return NULL; else if (fh_len !=3D 3) return NULL; So, we took care of these '2' conditions within the switch statement based on the 'fh_type'. We can just change the comparision condition from '<' to '!=3D': switch (fh_type) { + case FILEID_FAT_WITHOUT_PARENT: + if (fh_len !=3D FAT_FID_SIZE_WITHOUT_PARENT) + return NULL; + case FILEID_FAT_WITH_PARENT: + if ((fh_len !=3D FAT_FID_SIZE_WITH_PARENT) && + (fh_type =3D=3D FILEID_FAT_WITH_PARENT)) + return NULL; + i_pos =3D fid->i_pos_hi; + i_pos =3D (i_pos << 32) | (fid->i_pos_low); + inode =3D __fat_nfs_get_inode(sb, 0, fid->i_gen, i_pos); + break; + } I think there is no need to push the comparision statements in the begining similar to generic_fh_to_dentry. Thanks OGAWA. > -- > OGAWA Hirofumi > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html