[linux-cifs-client] PATCH: alignment changes for CIFS VFS

Jeff Layton jlayton at redhat.com
Tue Jun 30 00:17:42 GMT 2009


On Mon, 2009-06-29 at 11:30 -0600, Craig Matsuura wrote:
> No comments regarding this patch?  I sent a more recent patch, but the
> moderator has not approve it apparently?  I was hoping to get some
> feedback as if anynone else has seen such problems with alignment.  I
> wanted to fix this in our kernel so there are not fixups.

A few general comments...

First, the patch was posted as HTML mail which pretty much makes it
impossible for anyone to use. It also has a lot of word-wrap munging
which further causes problems. I generally use git-format-patch and
git-send-email for sending patches. Send them to yourself first until
you get the hang of it and convince yourself that your mailer hasn't
messed it up.

The patch looks like it's based on an older kernel. If you want it in
mainline you'll need to update it to the latest. I recommend pulling
down Steve French's tree and making sure that it applies there since
that represents the bleeding edge CIFS code.

Aside from the word wrap, this kind of stuff needs be removed:

+/*printk("*pBCC=%x, get_unaligned=%x\n",(*(u16 *)pBCC),
get_unaligned((u16 *) 
pBCC));*/

...if you want to add debugging messages, use the cFYI macro.

Also, places where you commented out existing code and replaced it with
new code should just remove the old code.

Comments in the middle of a line like this are really ugly. Yes, CIFS
has tons of them, but let's avoid adding new ones:

+                                           (get_unaligned((u16 *)
((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 *
smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/ - 
1) / 2;

While you're at it, this whole get_unaligned construction really cries
out for a helper function or macro. This stuff was already hard to read
and you're making it even harder. Making it more readable would be a
good thing.

-- 
Jeff Layton <jlayton at redhat.com>



More information about the linux-cifs-client mailing list