Date: Mon, 2 Feb 2015 14:16:06 +0000
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
Message-ID: <20150202141606.GY29656@ZenIV.linux.org.uk>
References: <1422884203-27173-1-git-send-email-rupran@einserver.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1422884203-27173-1-git-send-email-rupran@einserver.de>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2260
Lines: 42

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).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/