From: Josh Triplett <josh@freedesktop.org>
To: Matthew Wilcox <matthew@wil.cx>
Cc: linux-sparse@vger.kernel.org
Subject: Re: Bogus 'not declared' warning
Date: Tue, 13 Nov 2007 04:13:32 -0800 [thread overview]
Message-ID: <473994EC.8080306@freedesktop.org> (raw)
In-Reply-To: <471E4A2B.3080500@freedesktop.org>
[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]
Josh Triplett wrote:
> Matthew Wilcox wrote:
>> Seems to me that sparse ignores 'static' forward declarations, leading
>> to false warnings like this:
>>
>> /home/willy/kernel/linux-2.6/drivers/scsi/sym53c8xx_2/sym_hipd.c:3786:5: warning: symbol 'sym_compute_residual' was not declared. Should it be static?
>>
>> $ grep -n sym_compute_residual drivers/scsi/sym53c8xx_2/*
>> drivers/scsi/sym53c8xx_2/sym_hipd.c:61:static int sym_compute_residual(struct sym_hcb *np, struct sym_ccb *cp);
>> drivers/scsi/sym53c8xx_2/sym_hipd.c:3033: cp->sv_resid = sym_compute_residual(np, cp);
>> drivers/scsi/sym53c8xx_2/sym_hipd.c:3786:int sym_compute_residual(struct sym_hcb *np, struct sym_ccb *cp)
>
> Interesting; yes, it looks like the routine emitting that warning
> doesn't check for a symbol marked static by having a previous static
> declaration. That warning comes from check_duplicates in evaluate.c.
> Seeing it means Sparse didn't see any previous declaration of the
> symbol by checking the same_symbol linked list, which seems wrong.
> check_declaration in symbol.c hooks symbols into those lists. That
> implies 1) the scopes don't match and 2) one or the other symbol
> doesn't have extern. The latter should clearly hold true (that case,
> IIRC, handles letting you put extern declarations inside inner
> scopes). I think the former occurs because one declaration has file
> scope and the other one global scope, but that has a chicken-and-egg
> issue in the case of a static forward declaration: the static forward
> declaration applies, and makes the later declaration have file scope.
>
> I *think* the right fix involves changing check_declaration to check
> for this case specifically:
> if (next->scope == file_scope && sym->scope == global_scope) {
> sym->scope = file_scope;
> sym->same_symbol = next;
> return;
> }
Turns out this will take a bit more work to fix. check_declaration
seems like the wrong place for this check, and in any case the code
above won't properly hook the symbol into its scope since it doesn't
call bind_symbol. bind_symbol in turn has some dead code and other
problems. I don't plan to address this for the 0.4.1 release,
other than by adding a test case for this, because I don't want to
further delay the other bugfixes in 0.4.1.
- Josh Triplett
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]
prev parent reply other threads:[~2007-11-13 12:13 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-23 14:24 Bogus 'not declared' warning Matthew Wilcox
2007-10-23 19:23 ` Josh Triplett
2007-11-13 12:13 ` Josh Triplett [this message]
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=473994EC.8080306@freedesktop.org \
--to=josh@freedesktop.org \
--cc=linux-sparse@vger.kernel.org \
--cc=matthew@wil.cx \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).