| Message ID | 1394992972-23004-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Accepted |
| Headers | show |
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~
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
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
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~
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;
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(-)