On Thu, Feb 27, 2014 at 03:39:08PM -0500, Mark Hounschell wrote:
> On 02/26/2014 10:30 AM, Dan Carpenter wrote:
> > On Wed, Feb 26, 2014 at 10:18:26AM -0500, Mark Hounschell wrote:
> >> This patch addresses the follow error message followed
> >> by a kernel oops:
> >>
> >> dgap: driver does not set tty->port. This will crash the kernel later. Fix the driver
> >>
> >> It also renames the main function this patch addresses because
> >> its name is misleading.
> >>
> >
> > Thanks.
> >
> > regards,
> > dan carpenter
>
> I'm in the process of running dgap.c through checkpatch and creating another patch set. Before I get too far into it I wanted to get clarification on a couple of things.
Please line-wrap your emails :(
> 1. Should I wait until I know the status of my last patch before I post new ones?
I just now applied it :)
> 2. There are MANY "over 80 char lines" warnings. I'm uncertain of the acceptable way to fix some of them. Here is an example of one:
>
> while (tail != head) {
> if (reason & IFTLW) {
>
> if (ch->ch_tun.un_flags & UN_LOW) {
> ch->ch_tun.un_flags &= ~UN_LOW;
>
> // everything in this block is over 80 chars
>
> if (ch->ch_tun.un_flags & UN_ISOPEN) {
> if ((ch->ch_tun.un_tty->flags &
> (1 << TTY_DO_WRITE_WAKEUP)) &&
> ch->ch_tun.un_tty->ldisc->ops->write_wakeup) { // ????
> DGAP_UNLOCK(ch->ch_lock, lock_flags2);
> DGAP_UNLOCK(bd->bd_lock, lock_flags);
> (ch->ch_tun.un_tty->ldisc->ops->write_wakeup)(ch->ch_tun.un_tty); // ????
> DGAP_LOCK(bd->bd_lock, lock_flags);
> DGAP_LOCK(ch->ch_lock, lock_flags2);
> }
> wake_up_interruptible(&ch->ch_tun.un_tty->write_wait);
> wake_up_interruptible(&ch->ch_tun.un_flags_wait);
> // end of nasty block
>
> }
> }
>
> I figured I'd leave them for the last patch but there are a few that if I wait will show up in
> one or more of the patches preceding that last one. This one is actually one of them. While
> fixing up bracket errors with "chechpatch -file", chechpatch doesn't like the patch created.
For major logic chunks like this, I'd recommend just leaving them over
80 columns for now, until they can be refactored into something more
"readable" later. Don't try to just trail characters from the 70-80
character columns to make the tool happy, that's not a good idea.
Also, usually some of the if statement logic can be reversed to get the
code to be moved to the right easier, but maybe not.
good luck!
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
