From: "Jörn Engel" <joern@wohnheim.fh-wedel.de>
To: Jesper Juhl <juhl-lkml@dif.dk>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
Steve French <sfrench@samba.org>,
Steve French <sfrench@us.ibm.com>,
samba-technical <samba-technical@lists.samba.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [patch 2/3] whitespace cleanups in fs/cifs/file.c
Date: Wed, 29 Dec 2004 13:29:32 +0100 [thread overview]
Message-ID: <20041229122932.GC10308@wohnheim.fh-wedel.de> (raw)
In-Reply-To: <Pine.LNX.4.61.0412290347020.28589@dragon.hygekrogen.localhost>
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
next prev parent reply other threads:[~2004-12-29 12:30 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-12-26 23:24 [patch] copy_to_user check and whitespace cleanups in fs/cifs/file.c Jesper Juhl
2004-12-26 23:38 ` Alan Cox
2004-12-27 1:21 ` Jesper Juhl
2004-12-28 23:48 ` [patch 1/3] copy_to_user check " Jesper Juhl
2004-12-28 22:45 ` Alan Cox
2004-12-28 23:52 ` [patch 2/3] whitespace cleanups " Jesper Juhl
2004-12-29 1:57 ` Jörn Engel
2004-12-29 2:59 ` Jesper Juhl
2004-12-29 12:29 ` Jörn Engel [this message]
2004-12-29 18:48 ` Domen Puncer
2004-12-29 13:42 ` [patch 2/3] " Horst von Brand
2004-12-29 14:48 ` Jörn Engel
2004-12-29 16:31 ` Horst von Brand
2004-12-29 9:51 ` Wichert Akkerman
2004-12-29 12:10 ` Jörn Engel
2005-01-03 12:01 ` Martin Waitz
2004-12-28 23:55 ` [patch 3/3] get rid of two unnessesary assignments " Jesper Juhl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20041229122932.GC10308@wohnheim.fh-wedel.de \
--to=joern@wohnheim.fh-wedel.de \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=juhl-lkml@dif.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=samba-technical@lists.samba.org \
--cc=sfrench@samba.org \
--cc=sfrench@us.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).