[linux-cifs-client] PATCH: alignment changes for CIFS VFS
Craig Matsuura
cmatsuura at control4.com
Tue Jun 30 00:31:52 GMT 2009
Thanks for the comments. I consider a macro to replace the ugly, get_unaligned (Good idea). I was wonder if the change to using a get_unaligned was a
sound solution to the problem I was encountering. I'm sure this is not hi priority as this is not x86 specific it is more of a problem on the ARM. I would only
want the changes in the mainly if you thought they made sense.
I can work on cleaning up the debugs printk's (sorry about that). And I could add a macro. You are correct about the older kernel, our device is based on a
2.6.28.10. I do have a few questions.
I am unfamiliar with cFYI macro and how it is used, I guess I can just google. Is there a better place?
As for git formatted patches I am not familiar with the git patch creation. Where is the best place to learn about git patch creation?
It has been a while since I contributed anything to community so I apologize for the html and ugly patch. (Hope I got it fixed this time).
Thanks,
Craig
On Monday 29 June 2009 6:17:42 pm Jeff Layton wrote:
> 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.
--
Craig Matsuura - Principal Engineer
Control4
11734 South Election Road - Suite 200
Salt Lake City, UT 84020-6432
PH: 801-523-3161
FX: 801-523-3199
More information about the linux-cifs-client
mailing list