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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux