Re: PATCH: alignment changes for CIFS VFS

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