[PATCH 0/4] factor dirent handling

[PATCH 0/4] factor dirent handling

[Christoph Hellwig]

Introduce a common cifs_dirent structure to abstract away the code
dealing with directory entries from the various on the wire forms.

[PATCH 1/4] cifs: cleanup cifs_filldir

[Christoph Hellwig]

Use sensible variable names and formatting and remove some superflous
checks on entry.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>

Index: linux-2.6/fs/cifs/readdir.c
===================================================================
--- linux-2.6.orig/fs/cifs/readdir.c	2011-05-22 13:13:40.748945551 +0200
+++ linux-2.6/fs/cifs/readdir.c	2011-05-22 13:16:29.743946544 +0200
@@ -681,57 +681,49 @@ static int cifs_get_name_from_search_buf
 	return rc;
 }
 
-static int cifs_filldir(char *pfindEntry, struct file *file, filldir_t filldir,
-			void *direntry, char *scratch_buf, unsigned int max_len)
+static int cifs_filldir(char *find_entry, struct file *file, filldir_t filldir,
+		void *dirent, char *scratch_buf, unsigned int max_len)
 {
-	int rc = 0;
-	struct qstr qstring;
-	struct cifsFileInfo *pCifsF;
-	u64    inum;
-	ino_t  ino;
-	struct super_block *sb;
-	struct cifs_sb_info *cifs_sb;
-	struct dentry *tmp_dentry;
+	struct cifsFileInfo *file_info = file->private_data;
+	struct super_block *sb = file->f_path.dentry->d_sb;
+	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
 	struct cifs_fattr fattr;
+	struct dentry *dentry;
+	struct qstr name;
+	int rc = 0;
+	u64 inum;
+	ino_t ino;
 
-	/* get filename and len into qstring */
-	/* get dentry */
-	/* decide whether to create and populate ionde */
-	if ((direntry == NULL) || (file == NULL))
-		return -EINVAL;
-
-	pCifsF = file->private_data;
-
-	if ((scratch_buf == NULL) || (pfindEntry == NULL) || (pCifsF == NULL))
-		return -ENOENT;
-
-	rc = cifs_entry_is_dot(pfindEntry, pCifsF);
 	/* skip . and .. since we added them first */
+	rc = cifs_entry_is_dot(find_entry, file_info);
 	if (rc != 0)
 		return 0;
 
-	sb = file->f_path.dentry->d_sb;
-	cifs_sb = CIFS_SB(sb);
-
-	qstring.name = scratch_buf;
-	rc = cifs_get_name_from_search_buf(&qstring, pfindEntry,
-			pCifsF->srch_inf.info_level,
-			pCifsF->srch_inf.unicode, cifs_sb,
-			max_len, &inum /* returned */);
-
+	name.name = scratch_buf;
+	rc = cifs_get_name_from_search_buf(&name, find_entry,
+					   file_info->srch_inf.info_level,
+					   file_info->srch_inf.unicode,
+					   cifs_sb, max_len, &inum);
 	if (rc)
 		return rc;
 
-	if (pCifsF->srch_inf.info_level == SMB_FIND_FILE_UNIX)
+	switch (file_info->srch_inf.info_level) {
+	case SMB_FIND_FILE_UNIX:
 		cifs_unix_basic_to_fattr(&fattr,
-				 &((FILE_UNIX_INFO *) pfindEntry)->basic,
-				 cifs_sb);
-	else if (pCifsF->srch_inf.info_level == SMB_FIND_FILE_INFO_STANDARD)
-		cifs_std_info_to_fattr(&fattr, (FIND_FILE_STANDARD_INFO *)
-					pfindEntry, cifs_sb);
-	else
-		cifs_dir_info_to_fattr(&fattr, (FILE_DIRECTORY_INFO *)
-					pfindEntry, cifs_sb);
+					 &((FILE_UNIX_INFO *)find_entry)->basic,
+					 cifs_sb);
+		break;
+	case SMB_FIND_FILE_INFO_STANDARD:
+		cifs_std_info_to_fattr(&fattr,
+				       (FIND_FILE_STANDARD_INFO *)find_entry,
+				       cifs_sb);
+		break;
+	default:
+		cifs_dir_info_to_fattr(&fattr,
+				       (FILE_DIRECTORY_INFO *)find_entry,
+				       cifs_sb);
+		break;
+	}
 
 	if (inum && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
 		fattr.cf_uniqueid = inum;
@@ -750,12 +742,12 @@ static int cifs_filldir(char *pfindEntry
 		fattr.cf_flags |= CIFS_FATTR_NEED_REVAL;
 
 	ino = cifs_uniqueid_to_ino_t(fattr.cf_uniqueid);
-	tmp_dentry = cifs_readdir_lookup(file->f_dentry, &qstring, &fattr);
+	dentry = cifs_readdir_lookup(file->f_dentry, &name, &fattr);
 
-	rc = filldir(direntry, qstring.name, qstring.len, file->f_pos,
-		     ino, fattr.cf_dtype);
+	rc = filldir(dirent, name.name, name.len, file->f_pos, ino,
+		     fattr.cf_dtype);
 
-	dput(tmp_dentry);
+	dput(dentry);
 	return rc;
 }
 

[PATCH 2/4] cifs: introduce cifs_dirent

[Christoph Hellwig]

Introduce a generic directory entry structure, and factor the parsing
of the various on the wire structures that can represent one into
a common helper.  Switch cifs_entry_is_dot over to use it as a start.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>

Index: linux-2.6/fs/cifs/readdir.c
===================================================================
--- linux-2.6.orig/fs/cifs/readdir.c	2011-05-22 13:16:29.743946544 +0200
+++ linux-2.6/fs/cifs/readdir.c	2011-05-22 13:23:51.096946749 +0200
@@ -4,6 +4,7 @@
  *   Directory search handling
  *
  *   Copyright (C) International Business Machines  Corp., 2004, 2008
+ *   Copyright (C) Red Hat, Inc., 2011
  *   Author(s): Steve French (sfrench-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org)
  *
  *   This library is free software; you can redistribute it and/or modify
@@ -290,10 +291,10 @@ error_exit:
 }
 
 /* return length of unicode string in bytes */
-static int cifs_unicode_bytelen(char *str)
+static int cifs_unicode_bytelen(const char *str)
 {
 	int len;
-	__le16 *ustr = (__le16 *)str;
+	const __le16 *ustr = (const __le16 *)str;
 
 	for (len = 0; len <= PATH_MAX; len++) {
 		if (ustr[len] == 0)
@@ -334,78 +335,128 @@ static char *nxt_dir_entry(char *old_ent
 
 }
 
+struct cifs_dirent {
+	const char	*name;
+	size_t		namelen;
+	u32		resume_key;
+	u64		ino;
+};
+
+static void cifs_fill_dirent_unix(struct cifs_dirent *de,
+		const FILE_UNIX_INFO *info, bool is_unicode)
+{
+	de->name = &info->FileName[0];
+	if (is_unicode)
+		de->namelen = cifs_unicode_bytelen(de->name);
+	else
+		de->namelen = strnlen(de->name, PATH_MAX);
+	de->resume_key = info->ResumeKey;
+	de->ino = le64_to_cpu(info->basic.UniqueId);
+}
+
+static void cifs_fill_dirent_dir(struct cifs_dirent *de,
+		const FILE_DIRECTORY_INFO *info)
+{
+	de->name = &info->FileName[0];
+	de->namelen = le32_to_cpu(info->FileNameLength);
+	de->resume_key = info->FileIndex;
+}
+
+static void cifs_fill_dirent_full(struct cifs_dirent *de,
+		const FILE_FULL_DIRECTORY_INFO *info)
+{
+	de->name = &info->FileName[0];
+	de->namelen = le32_to_cpu(info->FileNameLength);
+	de->resume_key = info->FileIndex;
+}
+
+static void cifs_fill_dirent_search(struct cifs_dirent *de,
+		const SEARCH_ID_FULL_DIR_INFO *info)
+{
+	de->name = &info->FileName[0];
+	de->namelen = le32_to_cpu(info->FileNameLength);
+	de->resume_key = info->FileIndex;
+	de->ino = le64_to_cpu(info->UniqueId);
+}
+
+static void cifs_fill_dirent_both(struct cifs_dirent *de,
+		const FILE_BOTH_DIRECTORY_INFO *info)
+{
+	de->name = &info->FileName[0];
+	de->namelen = le32_to_cpu(info->FileNameLength);
+	de->resume_key = info->FileIndex;
+}
+
+static void cifs_fill_dirent_std(struct cifs_dirent *de,
+		const FIND_FILE_STANDARD_INFO *info)
+{
+	de->name = &info->FileName[0];
+	/* one byte length, no endianess conversion */
+	de->namelen = info->FileNameLength;
+	de->resume_key = info->ResumeKey;
+}
+
+static int cifs_fill_dirent(struct cifs_dirent *de, const void *info,
+		u16 level, bool is_unicode)
+{
+	memset(de, 0, sizeof(*de));
+
+	switch (level) {
+	case SMB_FIND_FILE_UNIX:
+		cifs_fill_dirent_unix(de, info, is_unicode);
+		break;
+	case SMB_FIND_FILE_DIRECTORY_INFO:
+		cifs_fill_dirent_dir(de, info);
+		break;
+	case SMB_FIND_FILE_FULL_DIRECTORY_INFO:
+		cifs_fill_dirent_full(de, info);
+		break;
+	case SMB_FIND_FILE_ID_FULL_DIR_INFO:
+		cifs_fill_dirent_search(de, info);
+		break;
+	case SMB_FIND_FILE_BOTH_DIRECTORY_INFO:
+		cifs_fill_dirent_both(de, info);
+		break;
+	case SMB_FIND_FILE_INFO_STANDARD:
+		cifs_fill_dirent_std(de, info);
+		break;
+	default:
+		cFYI(1, "Unknown findfirst level %d", level);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 #define UNICODE_DOT cpu_to_le16(0x2e)
 
 /* return 0 if no match and 1 for . (current directory) and 2 for .. (parent) */
-static int cifs_entry_is_dot(char *current_entry, struct cifsFileInfo *cfile)
+static int cifs_entry_is_dot(struct cifs_dirent *de, bool is_unicode)
 {
 	int rc = 0;
-	char *filename = NULL;
-	int len = 0;
 
-	if (cfile->srch_inf.info_level == SMB_FIND_FILE_UNIX) {
-		FILE_UNIX_INFO *pFindData = (FILE_UNIX_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		if (cfile->srch_inf.unicode) {
-			len = cifs_unicode_bytelen(filename);
-		} else {
-			/* BB should we make this strnlen of PATH_MAX? */
-			len = strnlen(filename, 5);
-		}
-	} else if (cfile->srch_inf.info_level == SMB_FIND_FILE_DIRECTORY_INFO) {
-		FILE_DIRECTORY_INFO *pFindData =
-			(FILE_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (cfile->srch_inf.info_level ==
-			SMB_FIND_FILE_FULL_DIRECTORY_INFO) {
-		FILE_FULL_DIRECTORY_INFO *pFindData =
-			(FILE_FULL_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (cfile->srch_inf.info_level ==
-			SMB_FIND_FILE_ID_FULL_DIR_INFO) {
-		SEARCH_ID_FULL_DIR_INFO *pFindData =
-			(SEARCH_ID_FULL_DIR_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (cfile->srch_inf.info_level ==
-			SMB_FIND_FILE_BOTH_DIRECTORY_INFO) {
-		FILE_BOTH_DIRECTORY_INFO *pFindData =
-			(FILE_BOTH_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (cfile->srch_inf.info_level == SMB_FIND_FILE_INFO_STANDARD) {
-		FIND_FILE_STANDARD_INFO *pFindData =
-			(FIND_FILE_STANDARD_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = pFindData->FileNameLength;
-	} else {
-		cFYI(1, "Unknown findfirst level %d",
-			 cfile->srch_inf.info_level);
-	}
+	if (!de->name)
+		return 0;
 
-	if (filename) {
-		if (cfile->srch_inf.unicode) {
-			__le16 *ufilename = (__le16 *)filename;
-			if (len == 2) {
-				/* check for . */
-				if (ufilename[0] == UNICODE_DOT)
-					rc = 1;
-			} else if (len == 4) {
-				/* check for .. */
-				if ((ufilename[0] == UNICODE_DOT)
-				   && (ufilename[1] == UNICODE_DOT))
-					rc = 2;
-			}
-		} else /* ASCII */ {
-			if (len == 1) {
-				if (filename[0] == '.')
-					rc = 1;
-			} else if (len == 2) {
-				if ((filename[0] == '.') && (filename[1] == '.'))
-					rc = 2;
-			}
+	if (is_unicode) {
+		__le16 *ufilename = (__le16 *)de->name;
+		if (de->namelen == 2) {
+			/* check for . */
+			if (ufilename[0] == UNICODE_DOT)
+				rc = 1;
+		} else if (de->namelen == 4) {
+			/* check for .. */
+			if (ufilename[0] == UNICODE_DOT &&
+			    ufilename[1] == UNICODE_DOT)
+				rc = 2;
+		}
+	} else /* ASCII */ {
+		if (de->namelen == 1) {
+			if (de->name[0] == '.')
+				rc = 1;
+		} else if (de->namelen == 2) {
+			if (de->name[0] == '.' && de->name[1] == '.')
+				rc = 2;
 		}
 	}
 
@@ -687,6 +738,7 @@ static int cifs_filldir(char *find_entry
 	struct cifsFileInfo *file_info = file->private_data;
 	struct super_block *sb = file->f_path.dentry->d_sb;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+	struct cifs_dirent de = { NULL, };
 	struct cifs_fattr fattr;
 	struct dentry *dentry;
 	struct qstr name;
@@ -694,9 +746,13 @@ static int cifs_filldir(char *find_entry
 	u64 inum;
 	ino_t ino;
 
+	rc = cifs_fill_dirent(&de, find_entry, file_info->srch_inf.info_level,
+			      file_info->srch_inf.unicode);
+	if (rc)
+		return rc;
+
 	/* skip . and .. since we added them first */
-	rc = cifs_entry_is_dot(find_entry, file_info);
-	if (rc != 0)
+	if (cifs_entry_is_dot(&de, file_info->srch_inf.unicode))
 		return 0;
 
 	name.name = scratch_buf;

[PATCH 3/4] cifs: use cifs_dirent to replace cifs_get_name_from_search_buf

[Christoph Hellwig]

This allows us to parse the on the wire structures only once in
cifs_filldir.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>

Index: linux-2.6/fs/cifs/readdir.c
===================================================================
--- linux-2.6.orig/fs/cifs/readdir.c	2011-05-22 13:32:40.456945333 +0200
+++ linux-2.6/fs/cifs/readdir.c	2011-05-22 13:33:38.060010807 +0200
@@ -656,82 +656,6 @@ static int find_cifs_entry(const int xid
 	return rc;
 }
 
-/* inode num, inode type and filename returned */
-static int cifs_get_name_from_search_buf(struct qstr *pqst,
-	char *current_entry, __u16 level, unsigned int unicode,
-	struct cifs_sb_info *cifs_sb, unsigned int max_len, __u64 *pinum)
-{
-	int rc = 0;
-	unsigned int len = 0;
-	char *filename;
-	struct nls_table *nlt = cifs_sb->local_nls;
-
-	*pinum = 0;
-
-	if (level == SMB_FIND_FILE_UNIX) {
-		FILE_UNIX_INFO *pFindData = (FILE_UNIX_INFO *)current_entry;
-
-		filename = &pFindData->FileName[0];
-		if (unicode) {
-			len = cifs_unicode_bytelen(filename);
-		} else {
-			/* BB should we make this strnlen of PATH_MAX? */
-			len = strnlen(filename, PATH_MAX);
-		}
-
-		*pinum = le64_to_cpu(pFindData->basic.UniqueId);
-	} else if (level == SMB_FIND_FILE_DIRECTORY_INFO) {
-		FILE_DIRECTORY_INFO *pFindData =
-			(FILE_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (level == SMB_FIND_FILE_FULL_DIRECTORY_INFO) {
-		FILE_FULL_DIRECTORY_INFO *pFindData =
-			(FILE_FULL_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (level == SMB_FIND_FILE_ID_FULL_DIR_INFO) {
-		SEARCH_ID_FULL_DIR_INFO *pFindData =
-			(SEARCH_ID_FULL_DIR_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-		*pinum = le64_to_cpu(pFindData->UniqueId);
-	} else if (level == SMB_FIND_FILE_BOTH_DIRECTORY_INFO) {
-		FILE_BOTH_DIRECTORY_INFO *pFindData =
-			(FILE_BOTH_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-	} else if (level == SMB_FIND_FILE_INFO_STANDARD) {
-		FIND_FILE_STANDARD_INFO *pFindData =
-			(FIND_FILE_STANDARD_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		/* one byte length, no name conversion */
-		len = (unsigned int)pFindData->FileNameLength;
-	} else {
-		cFYI(1, "Unknown findfirst level %d", level);
-		return -EINVAL;
-	}
-
-	if (len > max_len) {
-		cERROR(1, "bad search response length %d past smb end", len);
-		return -EINVAL;
-	}
-
-	if (unicode) {
-		pqst->len = cifs_from_ucs2((char *) pqst->name,
-					   (__le16 *) filename,
-					   UNICODE_NAME_MAX,
-					   min(len, max_len), nlt,
-					   cifs_sb->mnt_cifs_flags &
-						CIFS_MOUNT_MAP_SPECIAL_CHR);
-		pqst->len -= nls_nullsize(nlt);
-	} else {
-		pqst->name = filename;
-		pqst->len = len;
-	}
-	return rc;
-}
-
 static int cifs_filldir(char *find_entry, struct file *file, filldir_t filldir,
 		void *dirent, char *scratch_buf, unsigned int max_len)
 {
@@ -743,7 +667,6 @@ static int cifs_filldir(char *find_entry
 	struct dentry *dentry;
 	struct qstr name;
 	int rc = 0;
-	u64 inum;
 	ino_t ino;
 
 	rc = cifs_fill_dirent(&de, find_entry, file_info->srch_inf.info_level,
@@ -751,17 +674,31 @@ static int cifs_filldir(char *find_entry
 	if (rc)
 		return rc;
 
+	if (de.namelen > max_len) {
+		cERROR(1, "bad search response length %zd past smb end",
+			  de.namelen);
+		return -EINVAL;
+	}
+
 	/* skip . and .. since we added them first */
 	if (cifs_entry_is_dot(&de, file_info->srch_inf.unicode))
 		return 0;
 
-	name.name = scratch_buf;
-	rc = cifs_get_name_from_search_buf(&name, find_entry,
-					   file_info->srch_inf.info_level,
-					   file_info->srch_inf.unicode,
-					   cifs_sb, max_len, &inum);
-	if (rc)
-		return rc;
+	if (file_info->srch_inf.unicode) {
+		struct nls_table *nlt = cifs_sb->local_nls;
+
+		name.name = scratch_buf;
+		name.len =
+			cifs_from_ucs2((char *)name.name, (__le16 *)de.name,
+				       UNICODE_NAME_MAX,
+				       min(de.namelen, (size_t)max_len), nlt,
+				       cifs_sb->mnt_cifs_flags &
+					       	CIFS_MOUNT_MAP_SPECIAL_CHR);
+		name.len -= nls_nullsize(nlt);
+	} else {
+		name.name = de.name;
+		name.len = de.namelen;
+	}
 
 	switch (file_info->srch_inf.info_level) {
 	case SMB_FIND_FILE_UNIX:
@@ -781,8 +718,8 @@ static int cifs_filldir(char *find_entry
 		break;
 	}
 
-	if (inum && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
-		fattr.cf_uniqueid = inum;
+	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
+		fattr.cf_uniqueid = de.ino;
 	} else {
 		fattr.cf_uniqueid = iunique(sb, ROOT_I);
 		cifs_autodisable_serverino(cifs_sb);

[PATCH 4/4] cifs: use cifs_dirent in cifs_save_resume_key

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>

Index: linux-2.6/fs/cifs/readdir.c
===================================================================
--- linux-2.6.orig/fs/cifs/readdir.c	2011-06-30 21:47:45.366190671 +0200
+++ linux-2.6/fs/cifs/readdir.c	2011-06-30 21:48:42.076189966 +0200
@@ -478,66 +478,18 @@ static int is_dir_changed(struct file *f
 }
 
 static int cifs_save_resume_key(const char *current_entry,
-	struct cifsFileInfo *cifsFile)
+	struct cifsFileInfo *file_info)
 {
-	int rc = 0;
-	unsigned int len = 0;
-	__u16 level;
-	char *filename;
+	struct cifs_dirent de;
+	int rc;
 
-	if ((cifsFile == NULL) || (current_entry == NULL))
-		return -EINVAL;
-
-	level = cifsFile->srch_inf.info_level;
-
-	if (level == SMB_FIND_FILE_UNIX) {
-		FILE_UNIX_INFO *pFindData = (FILE_UNIX_INFO *)current_entry;
-
-		filename = &pFindData->FileName[0];
-		if (cifsFile->srch_inf.unicode) {
-			len = cifs_unicode_bytelen(filename);
-		} else {
-			/* BB should we make this strnlen of PATH_MAX? */
-			len = strnlen(filename, PATH_MAX);
-		}
-		cifsFile->srch_inf.resume_key = pFindData->ResumeKey;
-	} else if (level == SMB_FIND_FILE_DIRECTORY_INFO) {
-		FILE_DIRECTORY_INFO *pFindData =
-			(FILE_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-		cifsFile->srch_inf.resume_key = pFindData->FileIndex;
-	} else if (level == SMB_FIND_FILE_FULL_DIRECTORY_INFO) {
-		FILE_FULL_DIRECTORY_INFO *pFindData =
-			(FILE_FULL_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-		cifsFile->srch_inf.resume_key = pFindData->FileIndex;
-	} else if (level == SMB_FIND_FILE_ID_FULL_DIR_INFO) {
-		SEARCH_ID_FULL_DIR_INFO *pFindData =
-			(SEARCH_ID_FULL_DIR_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-		cifsFile->srch_inf.resume_key = pFindData->FileIndex;
-	} else if (level == SMB_FIND_FILE_BOTH_DIRECTORY_INFO) {
-		FILE_BOTH_DIRECTORY_INFO *pFindData =
-			(FILE_BOTH_DIRECTORY_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		len = le32_to_cpu(pFindData->FileNameLength);
-		cifsFile->srch_inf.resume_key = pFindData->FileIndex;
-	} else if (level == SMB_FIND_FILE_INFO_STANDARD) {
-		FIND_FILE_STANDARD_INFO *pFindData =
-			(FIND_FILE_STANDARD_INFO *)current_entry;
-		filename = &pFindData->FileName[0];
-		/* one byte length, no name conversion */
-		len = (unsigned int)pFindData->FileNameLength;
-		cifsFile->srch_inf.resume_key = pFindData->ResumeKey;
-	} else {
-		cFYI(1, "Unknown findfirst level %d", level);
-		return -EINVAL;
+	rc = cifs_fill_dirent(&de, current_entry, file_info->srch_inf.info_level,
+			      file_info->srch_inf.unicode);
+	if (!rc) {
+		file_info->srch_inf.presume_name = de.name;
+		file_info->srch_inf.resume_name_len = de.namelen;
+		file_info->srch_inf.resume_key = de.resume_key;
 	}
-	cifsFile->srch_inf.resume_name_len = len;
-	cifsFile->srch_inf.presume_name = filename;
 	return rc;
 }
 
Index: linux-2.6/fs/cifs/cifsglob.h
===================================================================
--- linux-2.6.orig/fs/cifs/cifsglob.h	2011-06-30 21:45:08.449525955 +0200
+++ linux-2.6/fs/cifs/cifsglob.h	2011-06-30 21:48:42.076189966 +0200
@@ -501,7 +501,7 @@ struct cifs_search_info {
 	char *ntwrk_buf_start;
 	char *srch_entries_start;
 	char *last_entry;
-	char *presume_name;
+	const char *presume_name;
 	unsigned int resume_name_len;
 	bool endOfSearch:1;
 	bool emptyDir:1;

Re: [PATCH 0/4] factor dirent handling

[Jeff Layton]

On Sat, 16 Jul 2011 15:23:34 -0400
Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> Introduce a common cifs_dirent structure to abstract away the code
> dealing with directory entries from the various on the wire forms.

Nice cleanup, Christoph. I've looked over the set and it looks good to me.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 3/4] cifs: use cifs_dirent to replace cifs_get_name_from_search_buf

[Jeff Layton]

On Sat, 16 Jul 2011 15:24:22 -0400
Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> This allows us to parse the on the wire structures only once in
> cifs_filldir.
> 
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> 
> Index: linux-2.6/fs/cifs/readdir.c
> ===================================================================
> --- linux-2.6.orig/fs/cifs/readdir.c	2011-05-22 13:32:40.456945333 +0200
> +++ linux-2.6/fs/cifs/readdir.c	2011-05-22 13:33:38.060010807 +0200
> @@ -656,82 +656,6 @@ static int find_cifs_entry(const int xid
>  	return rc;
>  }
>  
> -/* inode num, inode type and filename returned */
> -static int cifs_get_name_from_search_buf(struct qstr *pqst,
> -	char *current_entry, __u16 level, unsigned int unicode,
> -	struct cifs_sb_info *cifs_sb, unsigned int max_len, __u64 *pinum)
> -{
> -	int rc = 0;
> -	unsigned int len = 0;
> -	char *filename;
> -	struct nls_table *nlt = cifs_sb->local_nls;
> -
> -	*pinum = 0;
> -
> -	if (level == SMB_FIND_FILE_UNIX) {
> -		FILE_UNIX_INFO *pFindData = (FILE_UNIX_INFO *)current_entry;
> -
> -		filename = &pFindData->FileName[0];
> -		if (unicode) {
> -			len = cifs_unicode_bytelen(filename);
> -		} else {
> -			/* BB should we make this strnlen of PATH_MAX? */
> -			len = strnlen(filename, PATH_MAX);
> -		}
> -
> -		*pinum = le64_to_cpu(pFindData->basic.UniqueId);
> -	} else if (level == SMB_FIND_FILE_DIRECTORY_INFO) {
> -		FILE_DIRECTORY_INFO *pFindData =
> -			(FILE_DIRECTORY_INFO *)current_entry;
> -		filename = &pFindData->FileName[0];
> -		len = le32_to_cpu(pFindData->FileNameLength);
> -	} else if (level == SMB_FIND_FILE_FULL_DIRECTORY_INFO) {
> -		FILE_FULL_DIRECTORY_INFO *pFindData =
> -			(FILE_FULL_DIRECTORY_INFO *)current_entry;
> -		filename = &pFindData->FileName[0];
> -		len = le32_to_cpu(pFindData->FileNameLength);
> -	} else if (level == SMB_FIND_FILE_ID_FULL_DIR_INFO) {
> -		SEARCH_ID_FULL_DIR_INFO *pFindData =
> -			(SEARCH_ID_FULL_DIR_INFO *)current_entry;
> -		filename = &pFindData->FileName[0];
> -		len = le32_to_cpu(pFindData->FileNameLength);
> -		*pinum = le64_to_cpu(pFindData->UniqueId);
> -	} else if (level == SMB_FIND_FILE_BOTH_DIRECTORY_INFO) {
> -		FILE_BOTH_DIRECTORY_INFO *pFindData =
> -			(FILE_BOTH_DIRECTORY_INFO *)current_entry;
> -		filename = &pFindData->FileName[0];
> -		len = le32_to_cpu(pFindData->FileNameLength);
> -	} else if (level == SMB_FIND_FILE_INFO_STANDARD) {
> -		FIND_FILE_STANDARD_INFO *pFindData =
> -			(FIND_FILE_STANDARD_INFO *)current_entry;
> -		filename = &pFindData->FileName[0];
> -		/* one byte length, no name conversion */
> -		len = (unsigned int)pFindData->FileNameLength;
> -	} else {
> -		cFYI(1, "Unknown findfirst level %d", level);
> -		return -EINVAL;
> -	}
> -
> -	if (len > max_len) {
> -		cERROR(1, "bad search response length %d past smb end", len);
> -		return -EINVAL;
> -	}
> -
> -	if (unicode) {
> -		pqst->len = cifs_from_ucs2((char *) pqst->name,
> -					   (__le16 *) filename,
> -					   UNICODE_NAME_MAX,
> -					   min(len, max_len), nlt,
> -					   cifs_sb->mnt_cifs_flags &
> -						CIFS_MOUNT_MAP_SPECIAL_CHR);
> -		pqst->len -= nls_nullsize(nlt);
> -	} else {
> -		pqst->name = filename;
> -		pqst->len = len;
> -	}
> -	return rc;
> -}
> -
>  static int cifs_filldir(char *find_entry, struct file *file, filldir_t filldir,
>  		void *dirent, char *scratch_buf, unsigned int max_len)
>  {
> @@ -743,7 +667,6 @@ static int cifs_filldir(char *find_entry
>  	struct dentry *dentry;
>  	struct qstr name;
>  	int rc = 0;
> -	u64 inum;
>  	ino_t ino;
>  
>  	rc = cifs_fill_dirent(&de, find_entry, file_info->srch_inf.info_level,
> @@ -751,17 +674,31 @@ static int cifs_filldir(char *find_entry
>  	if (rc)
>  		return rc;
>  
> +	if (de.namelen > max_len) {
> +		cERROR(1, "bad search response length %zd past smb end",
> +			  de.namelen);
> +		return -EINVAL;
> +	}
> +
>  	/* skip . and .. since we added them first */
>  	if (cifs_entry_is_dot(&de, file_info->srch_inf.unicode))
>  		return 0;
>  
> -	name.name = scratch_buf;
> -	rc = cifs_get_name_from_search_buf(&name, find_entry,
> -					   file_info->srch_inf.info_level,
> -					   file_info->srch_inf.unicode,
> -					   cifs_sb, max_len, &inum);
> -	if (rc)
> -		return rc;
> +	if (file_info->srch_inf.unicode) {
> +		struct nls_table *nlt = cifs_sb->local_nls;
> +
> +		name.name = scratch_buf;
> +		name.len =
> +			cifs_from_ucs2((char *)name.name, (__le16 *)de.name,
> +				       UNICODE_NAME_MAX,
> +				       min(de.namelen, (size_t)max_len), nlt,
> +				       cifs_sb->mnt_cifs_flags &
> +					       	CIFS_MOUNT_MAP_SPECIAL_CHR);

	FWIW, the above line has some minor whitespace problems (spaces
	before tab). 

> +		name.len -= nls_nullsize(nlt);
> +	} else {
> +		name.name = de.name;
> +		name.len = de.namelen;
> +	}
>  
>  	switch (file_info->srch_inf.info_level) {
>  	case SMB_FIND_FILE_UNIX:
> @@ -781,8 +718,8 @@ static int cifs_filldir(char *find_entry
>  		break;
>  	}
>  
> -	if (inum && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> -		fattr.cf_uniqueid = inum;
> +	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> +		fattr.cf_uniqueid = de.ino;
>  	} else {
>  		fattr.cf_uniqueid = iunique(sb, ROOT_I);
>  		cifs_autodisable_serverino(cifs_sb);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>

Re: [PATCH 0/4] factor dirent handling

[Steve French]

On Tue, Jul 19, 2011 at 5:57 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Sat, 16 Jul 2011 15:23:34 -0400
> Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>
>> Introduce a common cifs_dirent structure to abstract away the code
>> dealing with directory entries from the various on the wire forms.
>
> Nice cleanup, Christoph. I've looked over the set and it looks good to me.
>
> Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Yes.  Makes readdir.c much cleaner.  I fixed up the whitespace problems
with patch3 and merged.


-- 
Thanks,

Steve