Re: [PATCH][0/6] cifs: readdir.c cleanup

[Steve French <smfrench@austin.rr.com>]

Jesper Juhl wrote:

>Hi Steve,
>
>Here's one more cleanup for a file in fs/cifs - readdir.c (i'm going to 
>follow the order you told me you'd prefer first, then do the remaining 
>files in arbitrary order).
>I'm going to send the patches inline to make it easy for others to comment 
>if they so choose, but since you had problems with inline patches from me 
>last time I've also placed them online for you :
>
>http://www.linuxtux.org/~juhl/kernel_patches/fs_cifs_readdir-whitespace-cleanup-1.patch
>http://www.linuxtux.org/~juhl/kernel_patches/fs_cifs_readdir-whitespace-cleanup-2.patch
>http://www.linuxtux.org/~juhl/kernel_patches/fs_cifs_readdir-whitespace-cleanup-3.patch
>http://www.linuxtux.org/~juhl/kernel_patches/fs_cifs_readdir-kfree-cleanup.patch
>http://www.linuxtux.org/~juhl/kernel_patches/fs_cifs_readdir-cast-cleanup.patch
>http://www.linuxtux.org/~juhl/kernel_patches/fs_cifs_readdir-whitespace-cleanup-final-bits.patch
>
>(listed in the order they apply)
>
>
>Short description of each patch will be in the email with that patch 
>inline that will follow shortly.
>
>
>  
>
The first looks fine.  I am part way through reviewing the second, and 
so far only found one change (see following) that I question.  I prefer 
to keep the local variables together without a blank line between them.  
Is there a global Linux style compliance issue here?  By the way, it is 
not common to use typedefs but you will see a few in this function since 
the network protocol specification describes the format of the wire 
protocol using them and it makes the structure names match the standard.

 static char *nxt_dir_entry(char *old_entry, char *end_of_smb)
 {
-	char * new_entry;
-	FILE_DIRECTORY_INFO * pDirInfo = (FILE_DIRECTORY_INFO *)old_entry;
+	char *new_entry;
+
+	FILE_DIRECTORY_INFO *pDirInfo = (FILE_DIRECTORY_INFO *)old_entry;

I will apply at least a few of them, but I am busy doing a high priority 
fix to handle split transact2 responses (which could cause an oops in ls 
to some servers so is high priority - although it only occurs on large 
directories, and if the server decides to send two transact responses 
for one request (which is not that common) and a search entry is split 
in certain ways across two SMB responses).