LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Andreas Ruprecht <rupran@einserver.de>
Cc: Oleg Drokin <oleg.drokin@intel.com>,
	Andreas Dilger <andreas.dilger@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	HPDD-discuss@ml01.01.org, devel@driverdev.osuosl.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: lustre: osc: Fix sparse warning about osc_init
Date: Mon, 2 Feb 2015 14:16:06 +0000	[thread overview]
Message-ID: <20150202141606.GY29656@ZenIV.linux.org.uk> (raw)
In-Reply-To: <1422884203-27173-1-git-send-email-rupran@einserver.de>

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).

  reply	other threads:[~2015-02-02 14:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-02 13:36 Andreas Ruprecht
2015-02-02 14:16 ` Al Viro [this message]
2015-02-02 19:13   ` Andreas Ruprecht
2015-02-05 16:30     ` use of opaque subject lines Al Viro
2015-02-05 16:57       ` Lad, Prabhakar
2015-02-05 17:57         ` Greg Kroah-Hartman
2015-02-05 18:22           ` Lad, Prabhakar
2015-02-05 19:32           ` Paul Bolle
2015-02-05 20:06             ` Greg Kroah-Hartman
2015-02-05 20:53               ` Joe Perches
2015-02-05 17:08       ` Joe Perches
2015-02-05 17:25         ` Dan Carpenter
2015-02-05 17:31           ` Joe Perches
2015-02-05 17:32             ` Joe Perches
2015-02-05 17:35               ` Dan Carpenter
2015-02-05 18:01                 ` Joe Perches
2015-02-05 18:26                   ` Dan Carpenter
2015-02-05 18:03         ` Greg Kroah-Hartman
2015-02-02 19:24   ` [PATCH] staging: lustre: osc: Make osc_init() static Andreas Ruprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150202141606.GY29656@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=HPDD-discuss@ml01.01.org \
    --cc=andreas.dilger@intel.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg.drokin@intel.com \
    --cc=rupran@einserver.de \
    --subject='Re: [PATCH] staging: lustre: osc: Fix sparse warning about osc_init' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).