[PATCH] staging: lustre: osc: Fix sparse warning about osc_init

[PATCH] staging: lustre: osc: Fix sparse warning about osc_init

[Andreas Ruprecht]

When running sparse on the osc/ subdirectory, it shows the
following warning:

andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/
[...]
drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning:
symbol 'osc_init' was not declared. Should it be static?
[...]

As this is the module init function, it can (and should) be declared
static.

Signed-off-by: Andreas Ruprecht <rupran@einserver.de>
---
 drivers/staging/lustre/lustre/osc/osc_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index b9450b9..0adfa70 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -3332,7 +3332,7 @@ extern struct lu_kmem_descr osc_caches[];
 extern spinlock_t osc_ast_guard;
 extern struct lock_class_key osc_ast_guard_class;
 
-int __init osc_init(void)
+static int __init osc_init(void)
 {
 	struct lprocfs_static_vars lvars = { NULL };
 	int rc;
-- 
1.9.1

Re: [PATCH] staging: lustre: osc: Fix sparse warning about osc_init

[Al Viro]

On Mon, Feb 02, 2015 at 02:36:43PM +0100, Andreas Ruprecht wrote:
> When running sparse on the osc/ subdirectory, it shows the
> following warning:
> 
> andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/
> [...]
> drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning:
> symbol 'osc_init' was not declared. Should it be static?
> [...]
> 
> As this is the module init function, it can (and should) be declared
> static.
> 
> Signed-off-by: Andreas Ruprecht <rupran@einserver.de>

For pity sake, folks, could you at least try to produce less meaningless
commit summaries?  "Fix sparse warning" says nothing whatsoever about
what's getting done.  The role of sparse in that is simple - it has
provided a tip to look at that declaration.  That's all; it might as well
had been offered by a guy puking at the next stall in the loo after big
party.  And no, sparse alone is not sufficient to prove that it could be
made static - it might be used somewhere else with explicit extern, its
declaration might've been in a header that didn't get included here, or
under slightly incorrect ifdefs, etc.  Such things need to be investigated
manually, not that it was hard to do.  OK, in this case the tip had panned
out; it can be made static (quick grep shows that) and it's better off
that way - less namespace pollution, makes life easier when doing analysis
of that code, etc.

_That_ is the point of this patch, not making the holy oracle shut the bleeding
fuck up.  If you want to credit sparse (or that guy puking in the next stall,
or whatever had drawn your attention to the function in question) - great,
do so inside the commit message.  _AFTER_ having described what the patch
does ("make osc_init() static" or equivalent thereof), please.  Ideally -
with something like "it's never used outside of osc_request.c" after summary
line (strictly speaking, something that serves as module_init might very well
be called elsewhere in the module explicitly; not common, but possible).

Re: [PATCH] staging: lustre: osc: Fix sparse warning about osc_init

[Andreas Ruprecht]

On 02.02.2015 15:16, Al Viro wrote:
> On Mon, Feb 02, 2015 at 02:36:43PM +0100, Andreas Ruprecht wrote:
>> When running sparse on the osc/ subdirectory, it shows the
>> following warning:
>>
>> andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/
>> [...]
>> drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning:
>> symbol 'osc_init' was not declared. Should it be static?
>> [...]
>>
>> As this is the module init function, it can (and should) be declared
>> static.
>>
>> Signed-off-by: Andreas Ruprecht <rupran@einserver.de>
> 
> For pity sake, folks, could you at least try to produce less meaningless
> commit summaries?  "Fix sparse warning" says nothing whatsoever about
> what's getting done.  The role of sparse in that is simple - it has
> provided a tip to look at that declaration.  That's all; it might as well
> had been offered by a guy puking at the next stall in the loo after big
> party.  And no, sparse alone is not sufficient to prove that it could be
> made static - it might be used somewhere else with explicit extern, its
> declaration might've been in a header that didn't get included here, or
> under slightly incorrect ifdefs, etc.  Such things need to be investigated
> manually, not that it was hard to do.  OK, in this case the tip had panned
> out; it can be made static (quick grep shows that) and it's better off
> that way - less namespace pollution, makes life easier when doing analysis
> of that code, etc.
> 
> _That_ is the point of this patch, not making the holy oracle shut the bleeding
> fuck up.  If you want to credit sparse (or that guy puking in the next stall,
> or whatever had drawn your attention to the function in question) - great,
> do so inside the commit message.  _AFTER_ having described what the patch
> does ("make osc_init() static" or equivalent thereof), please.  Ideally -
> with something like "it's never used outside of osc_request.c" after summary
> line (strictly speaking, something that serves as module_init might very well
> be called elsewhere in the module explicitly; not common, but possible).
> 

... and a very nice day to you, too, sir!

On a serious note: I do understand what you're getting at, I don't take
that personally (and I will send a v2 addressing the things above), but
honestly, this kind of answer might just be a real turn-off for other
people trying to get into kernel development...

I don't want to start a whole new 'attitude in the kernel community'
discussion, but I can't just let this go like that, sorry.

Regards,
Andreas

[PATCH] staging: lustre: osc: Make osc_init() static

[Andreas Ruprecht]

osc_init() is marked as the module_init function in osc_request.c
and is never used anywhere else. Hence, it can (and should) be
declared static.

sparse also complained about this with the following warning, which
is fixed by this patch.

andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/
[...]
drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning:
symbol 'osc_init' was not declared. Should it be static?
[...]

Signed-off-by: Andreas Ruprecht <rupran@einserver.de>
---
 drivers/staging/lustre/lustre/osc/osc_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index b9450b9..0adfa70 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -3332,7 +3332,7 @@ extern struct lu_kmem_descr osc_caches[];
 extern spinlock_t osc_ast_guard;
 extern struct lock_class_key osc_ast_guard_class;
 
-int __init osc_init(void)
+static int __init osc_init(void)
 {
 	struct lprocfs_static_vars lvars = { NULL };
 	int rc;
-- 
1.9.1

use of opaque subject lines

[Al Viro]

On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:

> On a serious note: I do understand what you're getting at, I don't take
> that personally (and I will send a v2 addressing the things above), but
> honestly, this kind of answer might just be a real turn-off for other
> people trying to get into kernel development...
> 
> I don't want to start a whole new 'attitude in the kernel community'
> discussion, but I can't just let this go like that, sorry.

Just during the last 12 hours or so, I've seen the following l-k traffic:

Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning
Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning
Subject: [PATCH] tty: vt/vt: fix sparse warning
Subject: [PATCH] scsi: fix sparse warnings
Subject: [PATCH] bfa: bfa_core: fix sparse warning
Subject: [PATCH] scsi: fix sparse warning
Subject: [PATCH] xen/acpi-processor: fix sparse warning
Subject: [PATCH] scsi: initio: fix sparse warnings
Subject: [PATCH] scsi: dc395x: fix sparse warning
Subject: [PATCH] scsi: eata: fix sparse warning
Subject: [PATCH] scsi: qla1280: fix sparse warnings
Subject: [PATCH] scsi: ips: fix sparse warnings
Subject: [PATCH] fbdev: via/via_clock: fix sparse warning
Subject: [PATCH] usb: gadget: fix sparse warnings
Subject: [PATCH] usb: gadget: fix sparse warnings
Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings
Subject: [PATCH] xen-netback: fix sparse warning
Subject: [PATCH] thermal: int340x: fix sparse warning
Subject: [PATCH] vxge: fix sparse warning
Subject: Re: [PATCH] xen-netback: fix sparse warning
Subject: [PATCH] ixgbe: fix sparse warnings
Subject: [PATCH] samsung-laptop: fix sparse warning
Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning
Subject: [PATCH] Sony-laptop: fix sparse warning

one of those is an ACK from maintainer, the rest are (AFAICS) all "make
something static", all with rather poor commit messages and subjects.  Not
threaded, either (that would've somewhat mitigated one of the problems).

Bad attitude?  I'd say that _this_ is one.  l-k is high-traffic list;
postings like that mean extra strain on those who are trying at least to
skim through it and they are not easy on those who are trying to read the
commit logs.  And it's trivial to mitigate - choose less opaque subjects,
for starters.  Because these are about as opaque as it gets: essentially,
it's "sparse had drawn my attention to $FILE; I've fixed that problem by
making it STFU".  Readers would like more information to filter upon?
Tough luck, open the damn mail and look into it.  One after another, after
another, to the tune of dozens per night.  All with the commit messages
pretty much of the form "$TOOL has produced $MESSAGE; with diff below it
doesn't".

It's physically impossible to read through all l-k traffic; the rate is too
high for that.  Is making it possible to do minimal triage too much to ask for?
I'm not blaming anyone for going after the low-hanging fruit like that to
get some experience on simple stuff, but as it is, it reinforces seriously
bad habits while creating considerable PITA for list readers.  And yes,
we'd all done our share of lousy commit messages, but this pretty much
teaches newbies to do that all the time.

Sigh...  Folks, please
	* Use descriptive subject lines at least somewhere in a thread.
	* Describe what the thing does, besides "makes $TOOL to STFU".
	* Remember, you don't fix warnings - you deal with the problem
(if any) in the code those warnings had pointed to.  And if there isn't
a problem (i.e. the warning was a false positive) you might be annotating
the code to tell the dumb tool what's going on there.  Which is a different
genre, and needs different mindset when reviewing (it's easy to obfuscate
a _real_ problem away - away from the $TOOL's ability to spot it, that
is).
	* Remember that said tools are nowhere near strong AI; the
same warning can come from very different underlying problems.  These
warnigns are heuristics - such-and-such looking code has high chances of
needing attention.  Give at least some indication what the problem _is_ - e.g.
the sparse warnings that has spawned this flood might come from
	1) function really is used only in the file it's defined in,
never intended to be used elsewhere, ought to be static to avoid namespace
pollution
	2) function is used elsewhere, and its uses elsewhere have explicit
externs right at the point of use.  Dangerous, since there's no practical
way for compiler (gcc, sparse, lint, whatever) to verify that there's no
type mismatch.  Ought to have extern placed in some header included both by
the file where it's defined and by all files where it's used, with externs
at the point of use removed.  Need to be careful about the choice of header
and location in it.
	3) function _is_ declared in a common header, but it's either not
included by the file where it actually lives, *or* is included only on
some architectures and/or configs.  Ought to figure out which one it is,
and either add an include or (needs _much_ more care) make some of
the includes in the chain of indirect includes unconditional.
	4) function is declared in a common header, and it is included,
but declaration is under the too tight set of ifdefs.  Might need to move
it around, but that also requires some care - less than what you need when
messing with conditional indirect includes, but also a non-trivial amount.
	Note that "I've made it static and result builds, so it must be
(1)" is incorrect - the code elsewhere actually using it might have been
ifdefed out on your config and architecture, simply not included into your
build, or optimized away (again, on your config and architecture).  "This
identifier is never mentioned elsewhere" is better (albeit still not
fool-proof - creative use of ## in macros might end up with something like
FOO_FUNC(add3) hiding a use of silly_prefix_add3_you_wont_find_me_with_grep,
and yes, things like that do happen).
	* When mass-submitting, use threading.  It's much easier on maillist
readers.
	* When using threading, at least try to keep the patches in one
thread related to each other.

Re: use of opaque subject lines

[Lad, Prabhakar]

On Thu, Feb 5, 2015 at 4:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:
>
>> On a serious note: I do understand what you're getting at, I don't take
>> that personally (and I will send a v2 addressing the things above), but
>> honestly, this kind of answer might just be a real turn-off for other
>> people trying to get into kernel development...
>>
>> I don't want to start a whole new 'attitude in the kernel community'
>> discussion, but I can't just let this go like that, sorry.
>
> Just during the last 12 hours or so, I've seen the following l-k traffic:
>
> Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning
> Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning
> Subject: [PATCH] tty: vt/vt: fix sparse warning
> Subject: [PATCH] scsi: fix sparse warnings
> Subject: [PATCH] bfa: bfa_core: fix sparse warning
> Subject: [PATCH] scsi: fix sparse warning
> Subject: [PATCH] xen/acpi-processor: fix sparse warning
> Subject: [PATCH] scsi: initio: fix sparse warnings
> Subject: [PATCH] scsi: dc395x: fix sparse warning
> Subject: [PATCH] scsi: eata: fix sparse warning
> Subject: [PATCH] scsi: qla1280: fix sparse warnings
> Subject: [PATCH] scsi: ips: fix sparse warnings
> Subject: [PATCH] fbdev: via/via_clock: fix sparse warning
> Subject: [PATCH] usb: gadget: fix sparse warnings
> Subject: [PATCH] usb: gadget: fix sparse warnings
> Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings
> Subject: [PATCH] xen-netback: fix sparse warning
> Subject: [PATCH] thermal: int340x: fix sparse warning
> Subject: [PATCH] vxge: fix sparse warning
> Subject: Re: [PATCH] xen-netback: fix sparse warning
> Subject: [PATCH] ixgbe: fix sparse warnings
> Subject: [PATCH] samsung-laptop: fix sparse warning
> Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning
> Subject: [PATCH] Sony-laptop: fix sparse warning
>
all right I have stopped the script to send any more patches fixing
sparse warnings !

Cheers,
--Prabhakar Lad

Re: use of opaque subject lines

[Joe Perches]

On Thu, 2015-02-05 at 16:30 +0000, Al Viro wrote:
> On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:
> 
> > On a serious note: I do understand what you're getting at, I don't take
> > that personally (and I will send a v2 addressing the things above), but
> > honestly, this kind of answer might just be a real turn-off for other
> > people trying to get into kernel development...
> > 
> > I don't want to start a whole new 'attitude in the kernel community'
> > discussion, but I can't just let this go like that, sorry.

Maybe YA checkpatch warning when patch subjects
include either "checkpatch" or "sparse" would help?

Something like:
---
 scripts/checkpatch.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3642b0d..b6bed59 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2170,6 +2170,13 @@ sub process {
 			}
 		}
 
+# Check email subject for poor style
+		if ($in_header_lines &&
+		    $line =~ /^Subject:.*\b(?:checkpatch|sparse)\b[^:]/i) {
+			WARN("EMAIL_SUBJECT",
+			     "A patch subject line should describe the change not the tool that found it\n" . $herecurr);
+		}
+
 # Check for old stable address
 		if ($line =~ /^\s*cc:\s*.*<?\bstable\@kernel\.org\b>?.*$/i) {
 			ERROR("STABLE_ADDRESS",

Re: use of opaque subject lines

[Dan Carpenter]

Acked-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: use of opaque subject lines

[Joe Perches]

On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote:
> Acked-by: Dan Carpenter <dan.carpenter@oracle.com>

Maybe I should add sparse too... ;)

Re: use of opaque subject lines

[Joe Perches]

On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote:
> On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote:
> > Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Maybe I should add sparse too... ;)

drat, of course I meant smatch...

Re: use of opaque subject lines

[Dan Carpenter]

On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote:
> On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote:
> > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote:
> > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > Maybe I should add sparse too... ;)
> 
> drat, of course I meant smatch...
>

No need.  Smatch users are awesome at writing changelogs.

regards,
dan carpenter 

Re: use of opaque subject lines

[Greg Kroah-Hartman]

On Thu, Feb 05, 2015 at 04:57:09PM +0000, Lad, Prabhakar wrote:
> On Thu, Feb 5, 2015 at 4:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:
> >
> >> On a serious note: I do understand what you're getting at, I don't take
> >> that personally (and I will send a v2 addressing the things above), but
> >> honestly, this kind of answer might just be a real turn-off for other
> >> people trying to get into kernel development...
> >>
> >> I don't want to start a whole new 'attitude in the kernel community'
> >> discussion, but I can't just let this go like that, sorry.
> >
> > Just during the last 12 hours or so, I've seen the following l-k traffic:
> >
> > Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning
> > Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning
> > Subject: [PATCH] tty: vt/vt: fix sparse warning
> > Subject: [PATCH] scsi: fix sparse warnings
> > Subject: [PATCH] bfa: bfa_core: fix sparse warning
> > Subject: [PATCH] scsi: fix sparse warning
> > Subject: [PATCH] xen/acpi-processor: fix sparse warning
> > Subject: [PATCH] scsi: initio: fix sparse warnings
> > Subject: [PATCH] scsi: dc395x: fix sparse warning
> > Subject: [PATCH] scsi: eata: fix sparse warning
> > Subject: [PATCH] scsi: qla1280: fix sparse warnings
> > Subject: [PATCH] scsi: ips: fix sparse warnings
> > Subject: [PATCH] fbdev: via/via_clock: fix sparse warning
> > Subject: [PATCH] usb: gadget: fix sparse warnings
> > Subject: [PATCH] usb: gadget: fix sparse warnings
> > Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings
> > Subject: [PATCH] xen-netback: fix sparse warning
> > Subject: [PATCH] thermal: int340x: fix sparse warning
> > Subject: [PATCH] vxge: fix sparse warning
> > Subject: Re: [PATCH] xen-netback: fix sparse warning
> > Subject: [PATCH] ixgbe: fix sparse warnings
> > Subject: [PATCH] samsung-laptop: fix sparse warning
> > Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning
> > Subject: [PATCH] Sony-laptop: fix sparse warning
> >
> all right I have stopped the script to send any more patches fixing
> sparse warnings !

That's not the point at all {sigh}

The point is, if you are going to do fixes, also provide a valid subject
line too.  Think of the people on the receiving end of your patch, they
are the most valuable and limited resource our community has right now.
You want to make it as _easy_ as possible for them to accept your
contribution.  If you don't provide enough information, or drown them in
redundancy, or crappy patches, they will just get frustrated and drop
them all on the floor.

And _NEVER_ have automated scripts create patches and send them out.  I
only know of ONE person/bot that gets away with this, and you are not
that person, sorry.  It it not a script on the receiving end of your
output, so don't use a script to create a mess for them to dig through.

I want these types of fixes, but make it easy for us to accept them, not
hard, like Al is pointing out in very vivid detail.  To respond to his
heartfelt plea and detailed instructions with a "fine, I'll just go
away!" is disrespectful.

greg k-h

Re: use of opaque subject lines

[Joe Perches]

On Thu, 2015-02-05 at 20:35 +0300, Dan Carpenter wrote:
> On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote:
> > On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote:
> > > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote:
> > > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > Maybe I should add sparse too... ;)
> > drat, of course I meant smatch...
> No need.  Smatch users are awesome at writing changelogs.

:) good point, kinda...

$ git log --format=oneline | grep -wi smatch | wc -l
141
$ git log --pretty=oneline | grep -wi sparse | wc -l
2383
$ git log --pretty=oneline | grep -Pi "\bcheckpatch\b[^:]" | wc -l
1637

Re: use of opaque subject lines

[Greg Kroah-Hartman]

On Thu, Feb 05, 2015 at 09:08:59AM -0800, Joe Perches wrote:
> On Thu, 2015-02-05 at 16:30 +0000, Al Viro wrote:
> > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:
> > 
> > > On a serious note: I do understand what you're getting at, I don't take
> > > that personally (and I will send a v2 addressing the things above), but
> > > honestly, this kind of answer might just be a real turn-off for other
> > > people trying to get into kernel development...
> > > 
> > > I don't want to start a whole new 'attitude in the kernel community'
> > > discussion, but I can't just let this go like that, sorry.
> 
> Maybe YA checkpatch warning when patch subjects
> include either "checkpatch" or "sparse" would help?
> 
> Something like:
> ---
>  scripts/checkpatch.pl | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3642b0d..b6bed59 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2170,6 +2170,13 @@ sub process {
>  			}
>  		}
>  
> +# Check email subject for poor style
> +		if ($in_header_lines &&
> +		    $line =~ /^Subject:.*\b(?:checkpatch|sparse)\b[^:]/i) {
> +			WARN("EMAIL_SUBJECT",
> +			     "A patch subject line should describe the change not the tool that found it\n" . $herecurr);
> +		}
> +
>  # Check for old stable address
>  		if ($line =~ /^\s*cc:\s*.*<?\bstable\@kernel\.org\b>?.*$/i) {
>  			ERROR("STABLE_ADDRESS",
> 


Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: use of opaque subject lines

[Lad, Prabhakar]

On Thu, Feb 5, 2015 at 5:57 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Feb 05, 2015 at 04:57:09PM +0000, Lad, Prabhakar wrote:
>> On Thu, Feb 5, 2015 at 4:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote:
>> >
>> >> On a serious note: I do understand what you're getting at, I don't take
>> >> that personally (and I will send a v2 addressing the things above), but
>> >> honestly, this kind of answer might just be a real turn-off for other
>> >> people trying to get into kernel development...
>> >>
>> >> I don't want to start a whole new 'attitude in the kernel community'
>> >> discussion, but I can't just let this go like that, sorry.
>> >
>> > Just during the last 12 hours or so, I've seen the following l-k traffic:
>> >
>> > Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning
>> > Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning
>> > Subject: [PATCH] tty: vt/vt: fix sparse warning
>> > Subject: [PATCH] scsi: fix sparse warnings
>> > Subject: [PATCH] bfa: bfa_core: fix sparse warning
>> > Subject: [PATCH] scsi: fix sparse warning
>> > Subject: [PATCH] xen/acpi-processor: fix sparse warning
>> > Subject: [PATCH] scsi: initio: fix sparse warnings
>> > Subject: [PATCH] scsi: dc395x: fix sparse warning
>> > Subject: [PATCH] scsi: eata: fix sparse warning
>> > Subject: [PATCH] scsi: qla1280: fix sparse warnings
>> > Subject: [PATCH] scsi: ips: fix sparse warnings
>> > Subject: [PATCH] fbdev: via/via_clock: fix sparse warning
>> > Subject: [PATCH] usb: gadget: fix sparse warnings
>> > Subject: [PATCH] usb: gadget: fix sparse warnings
>> > Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings
>> > Subject: [PATCH] xen-netback: fix sparse warning
>> > Subject: [PATCH] thermal: int340x: fix sparse warning
>> > Subject: [PATCH] vxge: fix sparse warning
>> > Subject: Re: [PATCH] xen-netback: fix sparse warning
>> > Subject: [PATCH] ixgbe: fix sparse warnings
>> > Subject: [PATCH] samsung-laptop: fix sparse warning
>> > Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning
>> > Subject: [PATCH] Sony-laptop: fix sparse warning
>> >
>> all right I have stopped the script to send any more patches fixing
>> sparse warnings !
>
> That's not the point at all {sigh}
>
> The point is, if you are going to do fixes, also provide a valid subject
> line too.  Think of the people on the receiving end of your patch, they
> are the most valuable and limited resource our community has right now.
> You want to make it as _easy_ as possible for them to accept your
> contribution.  If you don't provide enough information, or drown them in
> redundancy, or crappy patches, they will just get frustrated and drop
> them all on the floor.
>
> And _NEVER_ have automated scripts create patches and send them out.  I
> only know of ONE person/bot that gets away with this, and you are not
> that person, sorry.  It it not a script on the receiving end of your
> output, so don't use a script to create a mess for them to dig through.
>
> I want these types of fixes, but make it easy for us to accept them, not
> hard, like Al is pointing out in very vivid detail.  To respond to his
> heartfelt plea and detailed instructions with a "fine, I'll just go
> away!" is disrespectful.
>
Sorry for that.

I agree a proper a description is needed ideally, but all these days prior to my
patches, the subject  line was 'fix sparse warnings' for such patches,
that’s the
reason I picked it for my script. I understand people get annoyed seeing so many
continuous patches with same subject line, ill make my script a bit smarter
to have detailed explanation now on.

Just a side note I verify the patch created by the script and if its
OK then only I post it to ML.

Regards,
--Prabhakar Lad

Re: use of opaque subject lines

[Dan Carpenter]

On Thu, Feb 05, 2015 at 10:01:39AM -0800, Joe Perches wrote:
> On Thu, 2015-02-05 at 20:35 +0300, Dan Carpenter wrote:
> > On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote:
> > > On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote:
> > > > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote:
> > > > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > Maybe I should add sparse too... ;)
> > > drat, of course I meant smatch...
> > No need.  Smatch users are awesome at writing changelogs.
> 
> :) good point, kinda...
> 
> $ git log --format=oneline | grep -wi smatch | wc -l
> 141

Please add smatch to the list of tools.

regards,
dan carpenter

Re: use of opaque subject lines

[Paul Bolle]

On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote:
> And _NEVER_ have automated scripts create patches and send them out.  I
> only know of ONE person/bot that gets away with this, and you are not
> that person, sorry.  It it not a script on the receiving end of your
> output, so don't use a script to create a mess for them to dig through.

Mind if I ask which person/bot you have in mind? People are trying to
bot away the kconfig related messages I send out - which I think is a
very good idea - and I'd like to know what level that bot should reach
before people can get out of its way.

(I happen to think that the stuff I currently do is best done with a
human filter applied.)

Thanks,


Paul Bolle

Re: use of opaque subject lines

[Greg Kroah-Hartman]

On Thu, Feb 05, 2015 at 08:32:13PM +0100, Paul Bolle wrote:
> On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote:
> > And _NEVER_ have automated scripts create patches and send them out.  I
> > only know of ONE person/bot that gets away with this, and you are not
> > that person, sorry.  It it not a script on the receiving end of your
> > output, so don't use a script to create a mess for them to dig through.
> 
> Mind if I ask which person/bot you have in mind?

If you don't know that there is a bot doing patches, then that proves it
is so good that people do not realize it :)

> People are trying to
> bot away the kconfig related messages I send out - which I think is a
> very good idea - and I'd like to know what level that bot should reach
> before people can get out of its way.
> 
> (I happen to think that the stuff I currently do is best done with a
> human filter applied.)

Please stick with a human filter, that's the best thing possible.

thanks,

greg k-h

Re: use of opaque subject lines

[Joe Perches]

On Thu, 2015-02-05 at 12:06 -0800, Greg Kroah-Hartman wrote:
> On Thu, Feb 05, 2015 at 08:32:13PM +0100, Paul Bolle wrote:
> > On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote:
> > > And _NEVER_ have automated scripts create patches and send them out.  I
> > > only know of ONE person/bot that gets away with this, and you are not
> > > that person, sorry.  It it not a script on the receiving end of your
> > > output, so don't use a script to create a mess for them to dig through.
> > 
> > Mind if I ask which person/bot you have in mind?
> 
> If you don't know that there is a bot doing patches, then that proves it
> is so good that people do not realize it :)

Hmm.  That proves bots should be used to create patches.