[Qemu-devel] [PATCH] util: Add 'static' attribute to function implementation

[Qemu-devel] [PATCH] util: Add 'static' attribute to function implementation

[Stefan Weil]

The static code analyzer smatch complains because of a missing 'static'
attribute:

util/module.c:166:6: warning:
 symbol 'module_load' was not declared. Should it be static?

'static' is used in the forward declaration, but not in the implementation.
Add it there, too.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 util/module.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/module.c b/util/module.c
index 863a8a3..214effb 100644
--- a/util/module.c
+++ b/util/module.c
@@ -163,7 +163,7 @@ out:
 }
 #endif
 
-void module_load(module_init_type type)
+static void module_load(module_init_type type)
 {
 #ifdef CONFIG_MODULES
     char *fname = NULL;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] util: Add 'static' attribute to function implementation

[Richard Henderson]

On 03/16/2014 11:02 AM, Stefan Weil wrote:
> 'static' is used in the forward declaration, but not in the implementation.
> Add it there, too.

You might consider reporting this as a bug in the analyzer, since the static in
the forward declaration does apply to the definition.

That said, for style it's usually better to make them match, so,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH] util: Add 'static' attribute to function implementation

[Stefan Weil]

Am 16.03.2014 19:06, schrieb Richard Henderson:
> On 03/16/2014 11:02 AM, Stefan Weil wrote:
>> 'static' is used in the forward declaration, but not in the implementation.
>> Add it there, too.
> 
> You might consider reporting this as a bug in the analyzer, since the static in
> the forward declaration does apply to the definition.
> 
> That said, for style it's usually better to make them match, so,
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Yes, the forward declaration applies here. IMHO it helps human reviewers
if they can see directly that some function is only local, therefore I
prefer the attribute 'static' at both code locations.

We have a similar situation with other attributes, too. Smatch also
complains about missing QEMU_NORETURN in our implementation code (we add
it only in header files for global functions). Do you think it would be
good to make header and implementation match there, too?

Stefan

Re: [Qemu-devel] [PATCH] util: Add 'static' attribute to function implementation

[Richard Henderson]

On 03/16/2014 11:42 AM, Stefan Weil wrote:
> We have a similar situation with other attributes, too. Smatch also
> complains about missing QEMU_NORETURN in our implementation code (we add
> it only in header files for global functions). Do you think it would be
> good to make header and implementation match there, too?

Not without changing our CODING_STYLE.

I'm not really fond of

void attributes some_function(arg, arg, arg)

since its quite easy to wind up with too long lines.  It's one of the things I
really like about the gnu style

void attributes
some_function(arg, arg, arg)

layout.  (In addition to being an old codger who really liked to be able to do
"grep ^some_function *.c" to find the implementation.)


r~

Re: [Qemu-devel] [Qemu-trivial] [PATCH] util: Add 'static' attribute to function implementation

[Michael Tokarev]

16.03.2014 22:02, Stefan Weil wrote:
> The static code analyzer smatch complains because of a missing 'static'
> attribute:

Thanks, applied to -trivial.

/mjt