[PATCH 44/83] staging: brcm80211: replaced typedef si_t with struct si_pub

Roland Vossen rvossen at broadcom.com
Wed Jun 8 12:20:36 UTC 2011


Hi Julian,

> The reason I was pointing this out was that while foo() doesn't need
> to know what's in struct bar, it will help type safety and general
> code cleanliness to have some declaration of struct bar in the
> headers.

Mostly agree with you. It is cleaner to define all forward declarations 
in one header file.

> If we have a module that exports a set of functions in it's header
> file (say "bar.h") like:
>
> extern struct bar *get_bar();
> extern void foo(struct bar *bar);
>
> the struct bar is part of the API of the module, even if no caller
> ever uses it's internals. Therefore the header file should include the
> line:
>
> struct bar;
>
> so that users of the API don't have to declare it themselves to
> suppress the compiler warnings that would be generated otherwise.

Or the header file should include another header file with this 
declaration.

> My objection was to the struct declaration appearing at the top of
> wl_iw.c, rather than in the header file that defines the functions
> that use it.

Got it.

> In fact, looking a bit further around, it appears that this
> declaration is already in aiutils.h making it's appearance at the top
> of wl_iw.c completely redundant, especially given that the file in
> question appears to not use *any* functions that use the struct, and
> if it does, it should include aiutils.h rather than declaring the
> struct itself - and this isn't introducing extra coupling, this is
> merely including the header that defines the API for the code that
> actually uses this struct.

I just created a patch that removes all forward struct declarations from 
.c files. This patch will be part of a future patch set, I have added a 
'Reported-by: ' line with your name to the commit message.

Thanks, Roland.




More information about the devel mailing list