linux-kernel - Re: [PATCH] backlight: remove obsolete comment for ->state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180704103401.GB496@dell>
Date:   Wed, 4 Jul 2018 11:34:01 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     LKML <linux-kernel@...r.kernel.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Jani Nikula <jani.nikula@...el.com>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Jingoo Han <jingoohan1@...il.com>
Subject: Re: [PATCH] backlight: remove obsolete comment for ->state

On Wed, 04 Jul 2018, Daniel Vetter wrote:
> On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
> > On Wed, 04 Jul 2018, Lee Jones wrote:
> > 
> > > > Jani spotted this when reviewing my earlier patch to remove the driver
> > > > internal usage of this field in
> > > > 
> > > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
> > > > Author: Daniel Vetter <daniel.vetter@...ll.ch>
> > > > Date:   Wed Apr 25 19:42:52 2018 +0200
> > > > 
> > > >     backlight: Nuke BL_CORE_DRIVER1
> > > 
> > > FYI, sending patches like this is not a good idea.
> > > 
> > > I'll clean it up for you this time, but in future please send patches
> > > properly and place any additional comments you may have below the
> > > '---' line.
> > 
> > Ah, I see what you've tried to do.  This hurt my eyes! :)
> > 
> > It's more conventional to reference commits like:
> > 
> >   Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
> > 
> > Again, I'll make the amendment to avoid further confusion.
> 
> So the first mail doesn't even bother to explain what's
> objectionable

In the first instance it looked as though you'd copied and pasted `git
log`, which if you'd done so would have been obvious to you and would
have required no further explanation.

Also bear in mind that I took your standing within the kernel
community into consideration, so speaking to you like a n00b or going
into unnecessary detail could have been considered superfluous at best
and condescending at worst.

> the 2nd mail still says "This hurts my eyes!".

It certainly did, yes.

Usually taken to meaning that it was hard to read in these scenarios.

> Over whitespace in the commit message.

Nothing to do with white space.  It was the format by which you chose
to reference a previous commit.  Instead it looked like a patch
formatting error.  I have received patches pasted from `git log`
before, and this looked just the same.

Once I'd realised what was going on, I followed up to explain and
provided some feedback on what to do differently in future.

> This kind of stuff is why graphics people really don't enjoy contributing
> to the kernel at large. A friendly request to resend with the color choice
> adjusted would go a lot further.

I apologise if my brevity hurt your feelings.  I have 290 emails to
get though post-paternity leave before I can start to think about
getting some real/paid work done.

> > > > Cc: Jani Nikula <jani.nikula@...el.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
> > > > Cc: Lee Jones <lee.jones@...aro.org>
> > > > Cc: Daniel Thompson <daniel.thompson@...aro.org>
> > > > Cc: Jingoo Han <jingoohan1@...il.com>
> > > > ---
> > > >  include/linux/backlight.h | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > > index 7fbf0539e14a..0b5897446dca 100644
> > > > --- a/include/linux/backlight.h
> > > > +++ b/include/linux/backlight.h
> > > > @@ -79,7 +79,6 @@ struct backlight_properties {
> > > >  	/* Backlight type */
> > > >  	enum backlight_type type;
> > > >  	/* Flags used to signal drivers of state changes */
> > > > -	/* Upper 4 bits are reserved for driver internal use */
> > > >  	unsigned int state;
> > > >  
> > > >  #define BL_CORE_SUSPENDED	(1 << 0)	/* backlight is suspended */
> > > 
> > 
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog