Messages in this thread | | | Date | Wed, 29 Dec 2004 13:29:32 +0100 | From | Jörn Engel <> | Subject | Re: [patch 2/3] whitespace cleanups in fs/cifs/file.c |
| |
On Wed, 29 December 2004 03:59:55 +0100, Jesper Juhl wrote: > > > > @@ -408,7 +410,7 @@ cifs_close(struct inode *inode, struct f > > > struct cifs_sb_info *cifs_sb; > > > struct cifsTconInfo *pTcon; > > > struct cifsFileInfo *pSMBFile = > > > - (struct cifsFileInfo *) file->private_data; > > > + (struct cifsFileInfo *)file->private_data; > > > > struct cifsFileInfo *pSMBFile = file->private_data; > > > > Casting a typeless pointer is pointless. > > > This was a 'whitespace fixes only' patch. I have no problem with going > through the file and looking for pointless casts etc, but that would be a > sepperate patch.
Sure. I noticed it while going through your patch, that's all. If you find the time for a second patch, that would be nice. Casts are a very effective obfuscation method and most are pretty simple to avoid. Maybe I should check the kernel janitor list and add this point, if it doesn't exist yet.
> > > - if(file->f_dentry) { > > > - if(file->f_dentry->d_inode) { > > > + if (file->f_dentry) { > > > + if (file->f_dentry->d_inode) { > > > > if (file->f_dentry && file->f_dentry->d_inode) { > > > > There is too little context to see if this conversion is possible. > > And I'm too lazy to check myself. > > > I didn't check that either since that's not what this patch was about - it > was strictly formatting/whitespace cleanups and no code changes.
Yup. Same as above, except for the janitor list.
> - there was a lot of lines in there ;)
You can say that again, Mr. Hat!
> I made those changes since (again) both styles are used in the file, so to > make it consistent I had to choose one of the styles, and picked my > personal preference - that's the only reason behind that change.
Personal style is hard to argue about. And doesn't make much of a difference anyway.
> > > -static void reset_resume_key(struct file * dir_file, > > > - unsigned char * filename, > > > - unsigned int len,int Unicode,struct nls_table * nls_tab) { > > > +static void > > > +reset_resume_key(struct file *dir_file, unsigned char *filename, > > > + unsigned int len, int Unicode, struct nls_table *nls_tab) > > > +{ > > > > Lex Linus? Either way you don't stay within the 80 column. > > > Whoops, my bad, I intended to.
Sorry. The whole function declaration is spread over three lines. I don't mind p***ing Linus off iff putting the return type on a seperate line is sufficient to fit all the rest into a single line. Doesn't work here, so you get to argue in favor, not me. ;)
> Sepperate issue, sepperate patch.
Agreed. Google proposes "separate", btw.
Jörn
-- The strong give up and move away, while the weak give up and stay. -- unknown - 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/
| |