linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* automated warning notifications
@ 2012-06-14 17:25 Dan Carpenter
  2012-06-15  1:48 ` Fengguang Wu
  0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-14 17:25 UTC (permalink / raw)
  To: wfg; +Cc: linux-kernel, kernel-janitors

Hi Fengguang,

I also check new static checker warnings and sometimes email people.
I wonder if we are duplicating each others work.  For example, did
you send an email asking about the following Sparse warning:

sound/soc/codecs/ab8500-codec.c:1959:53: warning: cast truncates bits from constant value (1013 becomes 13)

Quite often those messages are false positive and the value is
truncated deliberately.  In this case it looks suspicious and I
would maybe email about it.  If I knew what kind of messages you
check and which you ignore that would help me.

Perhaps there is an email list I could subscribe to to see if you
had already sent a message.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-14 17:25 automated warning notifications Dan Carpenter
@ 2012-06-15  1:48 ` Fengguang Wu
  2012-06-15  7:12   ` Dan Carpenter
                     ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Fengguang Wu @ 2012-06-15  1:48 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-kernel, kernel-janitors

Hi Dan,

On Thu, Jun 14, 2012 at 08:25:23PM +0300, Dan Carpenter wrote:
> Hi Fengguang,
> 
> I also check new static checker warnings and sometimes email people.

That would be nice!

> I wonder if we are duplicating each others work.  For example, did
> you send an email asking about the following Sparse warning:
> 
> sound/soc/codecs/ab8500-codec.c:1959:53: warning: cast truncates bits from constant value (1013 becomes 13)

I did try sending out sparse warnings to people, some time ago...

> Quite often those messages are false positive and the value is
> truncated deliberately.  In this case it looks suspicious and I
> would maybe email about it.  If I knew what kind of messages you
> check and which you ignore that would help me.

The lots of false warnings are a big problem. It makes the automated
notification more noises than signals to people. So I end up disabling
the sparse check totally..

These days I only send out gcc errors/warnings to the commit author,
committer, signers and relevant mailing lists reported by
scripts/get_maintainer.pl (but minus LKML).

> Perhaps there is an email list I could subscribe to to see if you
> had already sent a message.

In an average working day, 1-2 build errors will be caught and email
notified. I guess there will be more sparse warnings if it's turned
on.

Perhaps the sparse warnings can be enabled, but only sent to the patch
author. If you and anyone else are interested, they could be sent to
some mailing list, too. One thing I'm sure is, we probably never want
to disturb the busy maintainers with these warnings.

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  1:48 ` Fengguang Wu
@ 2012-06-15  7:12   ` Dan Carpenter
  2012-06-15  7:58     ` Fengguang Wu
  2012-07-02 12:45   ` Dan Carpenter
  2012-07-06  3:07   ` Fengguang Wu
  2 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-15  7:12 UTC (permalink / raw)
  To: Fengguang Wu; +Cc: linux-kernel, kernel-janitors

On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> The lots of false warnings are a big problem. It makes the automated
> notification more noises than signals to people. So I end up disabling
> the sparse check totally..
> 

I do a basic sanity check of my emails before I send them.

Sometimes I do send false positives.  If the warning is introduced
by a very new code then probably the patch author can answer my
question off the top of her head.  Also I send some false positives
just to try learn what the rules are.

> In an average working day, 1-2 build errors will be caught and email
> notified. I guess there will be more sparse warnings if it's turned
> on.
> 
> Perhaps the sparse warnings can be enabled, but only sent to the patch
> author. If you and anyone else are interested, they could be sent to
> some mailing list, too. One thing I'm sure is, we probably never want
> to disturb the busy maintainers with these warnings.

Eventually I think we will want to set up a mailing list for this or
we will start sending duplicate messages.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  7:12   ` Dan Carpenter
@ 2012-06-15  7:58     ` Fengguang Wu
  2012-06-15  8:31       ` Josh Triplett
  2012-06-15 10:40       ` Julia Lawall
  0 siblings, 2 replies; 19+ messages in thread
From: Fengguang Wu @ 2012-06-15  7:58 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: linux-kernel, kernel-janitors, Pekka Enberg, Al Viro,
	Christopher Li, Josh Triplett, Linus Torvalds

[CC sparse developers]

// on setting up a list and sending newly found sparse warnings to it

On Fri, Jun 15, 2012 at 10:12:22AM +0300, Dan Carpenter wrote:
> On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> > The lots of false warnings are a big problem. It makes the automated
> > notification more noises than signals to people. So I end up disabling
> > the sparse check totally..
> > 
> 
> I do a basic sanity check of my emails before I send them.

Yeah that would be better.

> Sometimes I do send false positives.  If the warning is introduced
> by a very new code then probably the patch author can answer my
> question off the top of her head.  Also I send some false positives
> just to try learn what the rules are.

Heh.

> > In an average working day, 1-2 build errors will be caught and email
> > notified. I guess there will be more sparse warnings if it's turned
> > on.
> > 
> > Perhaps the sparse warnings can be enabled, but only sent to the patch
> > author. If you and anyone else are interested, they could be sent to
> > some mailing list, too. One thing I'm sure is, we probably never want
> > to disturb the busy maintainers with these warnings.
> 
> Eventually I think we will want to set up a mailing list for this or
> we will start sending duplicate messages.

Fair enough. How can we setup the mailing list? Once the list up, it
would be trivial for me to send sparse warnings out there.

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  7:58     ` Fengguang Wu
@ 2012-06-15  8:31       ` Josh Triplett
  2012-06-15  8:54         ` Fengguang Wu
  2012-06-15 10:40       ` Julia Lawall
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2012-06-15  8:31 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: Dan Carpenter, linux-kernel, kernel-janitors, Pekka Enberg,
	Al Viro, Christopher Li, Linus Torvalds

On Fri, Jun 15, 2012 at 03:58:10PM +0800, Fengguang Wu wrote:
> On Fri, Jun 15, 2012 at 10:12:22AM +0300, Dan Carpenter wrote:
> > On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> > > In an average working day, 1-2 build errors will be caught and email
> > > notified. I guess there will be more sparse warnings if it's turned
> > > on.
> > > 
> > > Perhaps the sparse warnings can be enabled, but only sent to the patch
> > > author. If you and anyone else are interested, they could be sent to
> > > some mailing list, too. One thing I'm sure is, we probably never want
> > > to disturb the busy maintainers with these warnings.
> > 
> > Eventually I think we will want to set up a mailing list for this or
> > we will start sending duplicate messages.
> 
> Fair enough. How can we setup the mailing list? Once the list up, it
> would be trivial for me to send sparse warnings out there.

Rather than a mailing list, how about something like test.kernel.org for
sparse warnings?

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  8:31       ` Josh Triplett
@ 2012-06-15  8:54         ` Fengguang Wu
  2012-06-15 16:48           ` Randy Dunlap
  0 siblings, 1 reply; 19+ messages in thread
From: Fengguang Wu @ 2012-06-15  8:54 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Dan Carpenter, linux-kernel, kernel-janitors, Pekka Enberg,
	Al Viro, Christopher Li, Linus Torvalds

On Fri, Jun 15, 2012 at 01:31:00AM -0700, Josh Triplett wrote:
> On Fri, Jun 15, 2012 at 03:58:10PM +0800, Fengguang Wu wrote:
> > On Fri, Jun 15, 2012 at 10:12:22AM +0300, Dan Carpenter wrote:
> > > On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> > > > In an average working day, 1-2 build errors will be caught and email
> > > > notified. I guess there will be more sparse warnings if it's turned
> > > > on.
> > > > 
> > > > Perhaps the sparse warnings can be enabled, but only sent to the patch
> > > > author. If you and anyone else are interested, they could be sent to
> > > > some mailing list, too. One thing I'm sure is, we probably never want
> > > > to disturb the busy maintainers with these warnings.
> > > 
> > > Eventually I think we will want to set up a mailing list for this or
> > > we will start sending duplicate messages.
> > 
> > Fair enough. How can we setup the mailing list? Once the list up, it
> > would be trivial for me to send sparse warnings out there.
> 
> Rather than a mailing list, how about something like test.kernel.org for
> sparse warnings?

It's much more trivial to send new build/sparse errors/warnings to a
list than to setup a website :-) As the errors come and go every day,
and they are mostly unstructured, it seems the mailing list would be a
more natural fit. People can search for known errors there and/or CC
fixes there.

Anyway, we just sent an request for creating

        automated-warnings@vger.kernel.org

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  7:58     ` Fengguang Wu
  2012-06-15  8:31       ` Josh Triplett
@ 2012-06-15 10:40       ` Julia Lawall
  2012-06-15 11:19         ` Dan Carpenter
  1 sibling, 1 reply; 19+ messages in thread
From: Julia Lawall @ 2012-06-15 10:40 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: Dan Carpenter, linux-kernel, kernel-janitors, Pekka Enberg,
	Al Viro, Christopher Li, Josh Triplett, Linus Torvalds

> > Eventually I think we will want to set up a mailing list for this or
> > we will start sending duplicate messages.
>
> Fair enough. How can we setup the mailing list? Once the list up, it
> would be trivial for me to send sparse warnings out there.

I'm not completely sure that a mailing list would completely eliminate
duplicate messages.  But still, it could be the place for people who are
interested in seeing such messages to go to, so it seems like a good
thing.  I would be happy to contribute content :)

julia

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 10:40       ` Julia Lawall
@ 2012-06-15 11:19         ` Dan Carpenter
  2012-06-15 13:33           ` Peter Senna Tschudin
  2012-06-15 14:34           ` Fengguang Wu
  0 siblings, 2 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-06-15 11:19 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Fengguang Wu, linux-kernel, kernel-janitors, Pekka Enberg,
	Al Viro, Christopher Li, Josh Triplett, Linus Torvalds

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

On Fri, Jun 15, 2012 at 06:40:51AM -0400, Julia Lawall wrote:
> > > Eventually I think we will want to set up a mailing list for this or
> > > we will start sending duplicate messages.
> >
> > Fair enough. How can we setup the mailing list? Once the list up, it
> > would be trivial for me to send sparse warnings out there.
> 
> I'm not completely sure that a mailing list would completely eliminate
> duplicate messages.  But still, it could be the place for people who are
> interested in seeing such messages to go to, so it seems like a good
> thing.  I would be happy to contribute content :)

Yeah.  That might be interesting.  If you don't know whether a bug
is a false positive or not you could submit it to the list for
people to look at.

I don't know if anyone will actually look at them.  I had been
planning to filter them to a mail box and automatically ignore
anything that was a duplicate.  But it might actually be worth
looking at them as well.  Especially if you email had enough useful
context so I could tell from the message what the bug is.

Probably we could use something like the attached script to print
out the line of code which causes the bug and some other script to
querry git blame and attach the offending commit?

regards,
dan carpenter


[-- Attachment #2: context.sh --]
[-- Type: application/x-sh, Size: 333 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 11:19         ` Dan Carpenter
@ 2012-06-15 13:33           ` Peter Senna Tschudin
  2012-06-15 13:53             ` Dan Carpenter
  2012-06-15 14:34           ` Fengguang Wu
  1 sibling, 1 reply; 19+ messages in thread
From: Peter Senna Tschudin @ 2012-06-15 13:33 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, Fengguang Wu, linux-kernel, kernel-janitors,
	Pekka Enberg, Al Viro, Christopher Li, Josh Triplett,
	Linus Torvalds

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

Added git-blame support...

On Fri, Jun 15, 2012 at 8:19 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Fri, Jun 15, 2012 at 06:40:51AM -0400, Julia Lawall wrote:
>> > > Eventually I think we will want to set up a mailing list for this or
>> > > we will start sending duplicate messages.
>> >
>> > Fair enough. How can we setup the mailing list? Once the list up, it
>> > would be trivial for me to send sparse warnings out there.
>>
>> I'm not completely sure that a mailing list would completely eliminate
>> duplicate messages.  But still, it could be the place for people who are
>> interested in seeing such messages to go to, so it seems like a good
>> thing.  I would be happy to contribute content :)
>
> Yeah.  That might be interesting.  If you don't know whether a bug
> is a false positive or not you could submit it to the list for
> people to look at.
>
> I don't know if anyone will actually look at them.  I had been
> planning to filter them to a mail box and automatically ignore
> anything that was a duplicate.  But it might actually be worth
> looking at them as well.  Especially if you email had enough useful
> context so I could tell from the message what the bug is.
>
> Probably we could use something like the attached script to print
> out the line of code which causes the bug and some other script to
> querry git blame and attach the offending commit?
>
> regards,
> dan carpenter
>



-- 
Peter Senna Tschudin
peter.senna@gmail.com
gpg id: 48274C36

[-- Attachment #2: context2.sh --]
[-- Type: application/x-sh, Size: 558 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 13:33           ` Peter Senna Tschudin
@ 2012-06-15 13:53             ` Dan Carpenter
  2012-06-15 15:49               ` Peter Senna Tschudin
  0 siblings, 1 reply; 19+ messages in thread
From: Dan Carpenter @ 2012-06-15 13:53 UTC (permalink / raw)
  To: Peter Senna Tschudin
  Cc: Julia Lawall, Fengguang Wu, linux-kernel, kernel-janitors, Josh Triplett

Dropped the Sparse people from the CC list.

On Fri, Jun 15, 2012 at 10:33:50AM -0300, Peter Senna Tschudin wrote:
> Added git-blame support...
> 

No.  I meant that it would run git blame on the offending line, take
the hash and then do a git show $hash.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 11:19         ` Dan Carpenter
  2012-06-15 13:33           ` Peter Senna Tschudin
@ 2012-06-15 14:34           ` Fengguang Wu
  2012-06-16  7:50             ` Cong Wang
  1 sibling, 1 reply; 19+ messages in thread
From: Fengguang Wu @ 2012-06-15 14:34 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, linux-kernel, kernel-janitors, Pekka Enberg,
	Al Viro, Christopher Li, Josh Triplett, Linus Torvalds

On Fri, Jun 15, 2012 at 02:19:14PM +0300, Dan Carpenter wrote:
> On Fri, Jun 15, 2012 at 06:40:51AM -0400, Julia Lawall wrote:
> > > > Eventually I think we will want to set up a mailing list for this or
> > > > we will start sending duplicate messages.
> > >
> > > Fair enough. How can we setup the mailing list? Once the list up, it
> > > would be trivial for me to send sparse warnings out there.
> > 
> > I'm not completely sure that a mailing list would completely eliminate
> > duplicate messages.  But still, it could be the place for people who are
> > interested in seeing such messages to go to, so it seems like a good
> > thing.  I would be happy to contribute content :)
> 
> Yeah.  That might be interesting.  If you don't know whether a bug
> is a false positive or not you could submit it to the list for
> people to look at.
> 
> I don't know if anyone will actually look at them.  I had been
> planning to filter them to a mail box and automatically ignore
> anything that was a duplicate.  But it might actually be worth
> looking at them as well.  Especially if you email had enough useful
> context so I could tell from the message what the bug is.
> 
> Probably we could use something like the attached script to print
> out the line of code which causes the bug and some other script to
> querry git blame and attach the offending commit?

cat -n $code_file | tail -n +$(($lineno - (($context + 1) / 2))) | head -n $(($context + 1))

That's handy, I'll use it to show the source file context for the
first error/warning :-)

Example:

drivers/leds/led-triggers.c: In function ‘led_trigger_event’:
drivers/leds/led-triggers.c:227:3: error: implicit declaration of function ‘led_set_brightness’ [-Werror=implicit-function-declaration]

drivers/leds/led-triggers.c:227:
   224                  struct led_classdev *led_cdev;
   225
   226                  led_cdev = list_entry(entry, struct led_classdev, trig_list);
 > 227                  led_set_brightness(led_cdev, brightness);
   228          }                                                
   229          read_unlock(&trigger->leddev_list_lock);
   230  }

Thanks!
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 13:53             ` Dan Carpenter
@ 2012-06-15 15:49               ` Peter Senna Tschudin
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Senna Tschudin @ 2012-06-15 15:49 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Julia Lawall, Fengguang Wu, linux-kernel, kernel-janitors, Josh Triplett

On Fri, Jun 15, 2012 at 10:53 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> Dropped the Sparse people from the CC list.
>
> On Fri, Jun 15, 2012 at 10:33:50AM -0300, Peter Senna Tschudin wrote:
>> Added git-blame support...
>>
>
> No.  I meant that it would run git blame on the offending line, take
> the hash and then do a git show $hash.

http://pastebin.com/cZCPQuzH

>
> regards,
> dan carpenter
>



-- 
Peter Senna Tschudin
peter.senna@gmail.com
gpg id: 48274C36

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  8:54         ` Fengguang Wu
@ 2012-06-15 16:48           ` Randy Dunlap
  2012-06-16  9:17             ` Fengguang Wu
  0 siblings, 1 reply; 19+ messages in thread
From: Randy Dunlap @ 2012-06-15 16:48 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: Josh Triplett, Dan Carpenter, linux-kernel, kernel-janitors,
	Pekka Enberg, Al Viro, Christopher Li, Linus Torvalds

On 06/15/2012 01:54 AM, Fengguang Wu wrote:

> On Fri, Jun 15, 2012 at 01:31:00AM -0700, Josh Triplett wrote:
>> On Fri, Jun 15, 2012 at 03:58:10PM +0800, Fengguang Wu wrote:
>>> On Fri, Jun 15, 2012 at 10:12:22AM +0300, Dan Carpenter wrote:
>>>> On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
>>>>> In an average working day, 1-2 build errors will be caught and email
>>>>> notified. I guess there will be more sparse warnings if it's turned
>>>>> on.
>>>>>
>>>>> Perhaps the sparse warnings can be enabled, but only sent to the patch
>>>>> author. If you and anyone else are interested, they could be sent to
>>>>> some mailing list, too. One thing I'm sure is, we probably never want
>>>>> to disturb the busy maintainers with these warnings.
>>>>
>>>> Eventually I think we will want to set up a mailing list for this or
>>>> we will start sending duplicate messages.
>>>
>>> Fair enough. How can we setup the mailing list? Once the list up, it
>>> would be trivial for me to send sparse warnings out there.
>>
>> Rather than a mailing list, how about something like test.kernel.org for
>> sparse warnings?
> 
> It's much more trivial to send new build/sparse errors/warnings to a
> list than to setup a website :-) As the errors come and go every day,
> and they are mostly unstructured, it seems the mailing list would be a
> more natural fit. People can search for known errors there and/or CC
> fixes there.
> 
> Anyway, we just sent an request for creating
> 
>         automated-warnings@vger.kernel.org


and you will let us know when it has been created??

Although I had just as soon use an existing list, like
kernel-janitors or kernel-testers.

thanks,
-- 
~Randy

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 14:34           ` Fengguang Wu
@ 2012-06-16  7:50             ` Cong Wang
  2012-06-16  9:01               ` Fengguang Wu
  0 siblings, 1 reply; 19+ messages in thread
From: Cong Wang @ 2012-06-16  7:50 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: Dan Carpenter, Julia Lawall, linux-kernel, kernel-janitors,
	Pekka Enberg, Al Viro, Christopher Li, Josh Triplett,
	Linus Torvalds

On Fri, Jun 15, 2012 at 10:34 PM, Fengguang Wu <wfg@linux.intel.com> wrote:
> On Fri, Jun 15, 2012 at 02:19:14PM +0300, Dan Carpenter wrote:
>>
>> Probably we could use something like the attached script to print
>> out the line of code which causes the bug and some other script to
>> querry git blame and attach the offending commit?
>
> cat -n $code_file | tail -n +$(($lineno - (($context + 1) / 2))) | head -n $(($context + 1))
>
> That's handy, I'll use it to show the source file context for the
> first error/warning :-)

Well, you can use sed/awk, it will be much shorter:

cat -n drivers/leds/led-triggers.c | sed -ne '224,230p'
   224			struct led_classdev *led_cdev;
   225	
   226			led_cdev = list_entry(entry, struct led_classdev, trig_list);
   227			led_set_brightness(led_cdev, brightness);
   228		}
   229		read_unlock(&trigger->leddev_list_lock);
   230	}

(replace the hard-coded "224,230" with a shell variable)

And if you want to find the offending commit:

git show `git blame drivers/leds/led-triggers.c | awk 'NR==227{print $1}'`

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-16  7:50             ` Cong Wang
@ 2012-06-16  9:01               ` Fengguang Wu
  0 siblings, 0 replies; 19+ messages in thread
From: Fengguang Wu @ 2012-06-16  9:01 UTC (permalink / raw)
  To: Cong Wang
  Cc: Dan Carpenter, Julia Lawall, linux-kernel, kernel-janitors,
	Josh Triplett, Randy Dunlap, Peter Senna Tschudin

[drop sparse developers]

On Sat, Jun 16, 2012 at 03:50:36PM +0800, Cong Wang wrote:
> On Fri, Jun 15, 2012 at 10:34 PM, Fengguang Wu <wfg@linux.intel.com> wrote:
> > On Fri, Jun 15, 2012 at 02:19:14PM +0300, Dan Carpenter wrote:
> >>
> >> Probably we could use something like the attached script to print
> >> out the line of code which causes the bug and some other script to
> >> querry git blame and attach the offending commit?
> >
> > cat -n $code_file | tail -n +$(($lineno - (($context + 1) / 2))) | head -n $(($context + 1))
> >
> > That's handy, I'll use it to show the source file context for the
> > first error/warning :-)
> 
> Well, you can use sed/awk, it will be much shorter:
> 
> cat -n drivers/leds/led-triggers.c | sed -ne '224,230p'
>    224			struct led_classdev *led_cdev;
>    225	
>    226			led_cdev = list_entry(entry, struct led_classdev, trig_list);
>    227			led_set_brightness(led_cdev, brightness);
>    228		}
>    229		read_unlock(&trigger->leddev_list_lock);
>    230	}
> 
> (replace the hard-coded "224,230" with a shell variable)

Thanks!  I'll use it this way:

lineno=227
context=3
cat -n drivers/leds/led-triggers.c | sed -e "s/  $lineno\t/> $lineno\t/" -ne "$((lineno - context)),$((lineno + context))p"
   224                  struct led_classdev *led_cdev;
   225
   226                  led_cdev = list_entry(entry, struct led_classdev, trig_list);
 > 227                  led_set_brightness(led_cdev, brightness);
   228          }
   229          read_unlock(&trigger->leddev_list_lock);
   230  }

> And if you want to find the offending commit:
> 
> git show `git blame drivers/leds/led-triggers.c | awk 'NR==227{print $1}'`

The code offered by Peter should run faster:

        hash=$(git blame $code_file -L $lineno,$lineno |cut -d " " -f 1)
        git --no-pager show $hash $code_file

Anyway, Dan's original idea of using git blame to find out the offending 
commit is valuable. The process can be automated like this:

1) use git-blame to find out the commit that changed $lineno in recent 100 days
2) checkout out $commit and build test and check build error
3) checkout out $commit~1 and build test and check build error

If (1) succeeded and (2) got the expected build error while (3) don't
have the build error, we reliably catch the bad commit. Otherwise the
script may try building test other commits that modified the file
recently.

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15 16:48           ` Randy Dunlap
@ 2012-06-16  9:17             ` Fengguang Wu
  2012-06-16 17:44               ` Randy Dunlap
  0 siblings, 1 reply; 19+ messages in thread
From: Fengguang Wu @ 2012-06-16  9:17 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Josh Triplett, Dan Carpenter, linux-kernel, kernel-janitors,
	Julia Lawall, Peter Senna Tschudin, Cong Wang

On Fri, Jun 15, 2012 at 09:48:26AM -0700, Randy Dunlap wrote:
> On 06/15/2012 01:54 AM, Fengguang Wu wrote:
> 
> > On Fri, Jun 15, 2012 at 01:31:00AM -0700, Josh Triplett wrote:
> >> On Fri, Jun 15, 2012 at 03:58:10PM +0800, Fengguang Wu wrote:
> >>> On Fri, Jun 15, 2012 at 10:12:22AM +0300, Dan Carpenter wrote:
> >>>> On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> >>>>> In an average working day, 1-2 build errors will be caught and email
> >>>>> notified. I guess there will be more sparse warnings if it's turned
> >>>>> on.
> >>>>>
> >>>>> Perhaps the sparse warnings can be enabled, but only sent to the patch
> >>>>> author. If you and anyone else are interested, they could be sent to
> >>>>> some mailing list, too. One thing I'm sure is, we probably never want
> >>>>> to disturb the busy maintainers with these warnings.
> >>>>
> >>>> Eventually I think we will want to set up a mailing list for this or
> >>>> we will start sending duplicate messages.
> >>>
> >>> Fair enough. How can we setup the mailing list? Once the list up, it
> >>> would be trivial for me to send sparse warnings out there.
> >>
> >> Rather than a mailing list, how about something like test.kernel.org for
> >> sparse warnings?
> > 
> > It's much more trivial to send new build/sparse errors/warnings to a
> > list than to setup a website :-) As the errors come and go every day,
> > and they are mostly unstructured, it seems the mailing list would be a
> > more natural fit. People can search for known errors there and/or CC
> > fixes there.
> > 
> > Anyway, we just sent an request for creating
> > 
> >         automated-warnings@vger.kernel.org
> 
> 
> and you will let us know when it has been created??

Well, the request has been rejected anyway..

> Although I had just as soon use an existing list, like
> kernel-janitors or kernel-testers.

>From http://kernelnewbies.org/KernelJanitors :

Some suggestions to kernel newbies:

        avoid fixing compiler warnings  because the goal is to fix the
        CAUSE of the warnings (which is usually not obvious), not just
        to make the warnings go away 

Does that suggest the commit author be the best people to fix
warnings? The typical situation may be, the author is not aware of the
warnings at all: they are buried in the tedious output of make...

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-16  9:17             ` Fengguang Wu
@ 2012-06-16 17:44               ` Randy Dunlap
  0 siblings, 0 replies; 19+ messages in thread
From: Randy Dunlap @ 2012-06-16 17:44 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: Josh Triplett, Dan Carpenter, linux-kernel, kernel-janitors,
	Julia Lawall, Peter Senna Tschudin, Cong Wang

On 06/16/2012 02:17 AM, Fengguang Wu wrote:

> On Fri, Jun 15, 2012 at 09:48:26AM -0700, Randy Dunlap wrote:
>> On 06/15/2012 01:54 AM, Fengguang Wu wrote:
>>
>>> On Fri, Jun 15, 2012 at 01:31:00AM -0700, Josh Triplett wrote:
>>>> On Fri, Jun 15, 2012 at 03:58:10PM +0800, Fengguang Wu wrote:
>>>>> On Fri, Jun 15, 2012 at 10:12:22AM +0300, Dan Carpenter wrote:
>>>>>> On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
>>>>>>> In an average working day, 1-2 build errors will be caught and email
>>>>>>> notified. I guess there will be more sparse warnings if it's turned
>>>>>>> on.
>>>>>>>
>>>>>>> Perhaps the sparse warnings can be enabled, but only sent to the patch
>>>>>>> author. If you and anyone else are interested, they could be sent to
>>>>>>> some mailing list, too. One thing I'm sure is, we probably never want
>>>>>>> to disturb the busy maintainers with these warnings.
>>>>>>
>>>>>> Eventually I think we will want to set up a mailing list for this or
>>>>>> we will start sending duplicate messages.
>>>>>
>>>>> Fair enough. How can we setup the mailing list? Once the list up, it
>>>>> would be trivial for me to send sparse warnings out there.
>>>>
>>>> Rather than a mailing list, how about something like test.kernel.org for
>>>> sparse warnings?
>>>
>>> It's much more trivial to send new build/sparse errors/warnings to a
>>> list than to setup a website :-) As the errors come and go every day,
>>> and they are mostly unstructured, it seems the mailing list would be a
>>> more natural fit. People can search for known errors there and/or CC
>>> fixes there.
>>>
>>> Anyway, we just sent an request for creating
>>>
>>>         automated-warnings@vger.kernel.org
>>
>>
>> and you will let us know when it has been created??
> 
> Well, the request has been rejected anyway..
> 
>> Although I had just as soon use an existing list, like
>> kernel-janitors or kernel-testers.
> 
> From http://kernelnewbies.org/KernelJanitors :
> 
> Some suggestions to kernel newbies:
> 
>         avoid fixing compiler warnings  because the goal is to fix the
>         CAUSE of the warnings (which is usually not obvious), not just
>         to make the warnings go away 
> 
> Does that suggest the commit author be the best people to fix
> warnings? The typical situation may be, the author is not aware of the
> warnings at all: they are buried in the tedious output of make...


It's a shame when a patch creates lots of warnings and they are
ignored.  I would suggest that the patch should not be merged.  :)

We should at least bring the warnings to the attention of the
patch author.  Sure, in some cases we (I) might make a patch that
the author wouldn't want and would have better solutions for.
That's OK.  It happens often.  It's part of how Linux development works.



-- 
~Randy

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  1:48 ` Fengguang Wu
  2012-06-15  7:12   ` Dan Carpenter
@ 2012-07-02 12:45   ` Dan Carpenter
  2012-07-06  3:07   ` Fengguang Wu
  2 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2012-07-02 12:45 UTC (permalink / raw)
  To: Fengguang Wu; +Cc: linux-kernel, kernel-janitors

On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> Hi Dan,
> 
> On Thu, Jun 14, 2012 at 08:25:23PM +0300, Dan Carpenter wrote:
> > Hi Fengguang,
> > 
> > I also check new static checker warnings and sometimes email people.
> 
> That would be nice!
> 
> > I wonder if we are duplicating each others work.  For example, did
> > you send an email asking about the following Sparse warning:
> > 
> > sound/soc/codecs/ab8500-codec.c:1959:53: warning: cast truncates bits from constant value (1013 becomes 13)
> 
> I did try sending out sparse warnings to people, some time ago...

Apparently you have started up again?  We're starting to trod on
each other's toes and people are getting annoyed with me because I
am reporting them after you...  :/

Randy has suggested that we send these automated messages to
kernel-janitors.  I don't know how the other people on kernel
janitors feel about this but I would be fine with it.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: automated warning notifications
  2012-06-15  1:48 ` Fengguang Wu
  2012-06-15  7:12   ` Dan Carpenter
  2012-07-02 12:45   ` Dan Carpenter
@ 2012-07-06  3:07   ` Fengguang Wu
  2 siblings, 0 replies; 19+ messages in thread
From: Fengguang Wu @ 2012-07-06  3:07 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-kernel, kernel-janitors, Randy Dunlap

> On Fri, Jun 15, 2012 at 09:48:35AM +0800, Fengguang Wu wrote:
> > Hi Dan,
> > 
> > On Thu, Jun 14, 2012 at 08:25:23PM +0300, Dan Carpenter wrote:
> > > Hi Fengguang,
> > > 
> > > I also check new static checker warnings and sometimes email people.
> > 
> > That would be nice!
> > 
> > > I wonder if we are duplicating each others work.  For example, did
> > > you send an email asking about the following Sparse warning:
> > > 
> > > sound/soc/codecs/ab8500-codec.c:1959:53: warning: cast truncates bits from constant value (1013 becomes 13)
> > 
> > I did try sending out sparse warnings to people, some time ago...
> 
> Apparently you have started up again?  We're starting to trod on
> each other's toes and people are getting annoyed with me because I
> am reporting them after you...  :/

Sorry for that!  Yeah I restarted the sparse checks and have been
sending out private emails after basic sanity check. Although there
are 10000 existing sparse warnings, there are only very few *new*
sparse warnings per day. This is pretty acceptable.

> Randy has suggested that we send these automated messages to
> kernel-janitors.  I don't know how the other people on kernel
> janitors feel about this but I would be fine with it.

Randy also suggested kernel-janitors. So let's try it. I'll CC
kernel-janitors for all of my error/warning reports and let's check
how well it works.

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2012-07-06  3:07 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-14 17:25 automated warning notifications Dan Carpenter
2012-06-15  1:48 ` Fengguang Wu
2012-06-15  7:12   ` Dan Carpenter
2012-06-15  7:58     ` Fengguang Wu
2012-06-15  8:31       ` Josh Triplett
2012-06-15  8:54         ` Fengguang Wu
2012-06-15 16:48           ` Randy Dunlap
2012-06-16  9:17             ` Fengguang Wu
2012-06-16 17:44               ` Randy Dunlap
2012-06-15 10:40       ` Julia Lawall
2012-06-15 11:19         ` Dan Carpenter
2012-06-15 13:33           ` Peter Senna Tschudin
2012-06-15 13:53             ` Dan Carpenter
2012-06-15 15:49               ` Peter Senna Tschudin
2012-06-15 14:34           ` Fengguang Wu
2012-06-16  7:50             ` Cong Wang
2012-06-16  9:01               ` Fengguang Wu
2012-07-02 12:45   ` Dan Carpenter
2012-07-06  3:07   ` Fengguang Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).