On Tue, 7 Jul 2015 10:18:47 -0700 "Jasper St. Pierre" <[email protected]> wrote:
> Wacky. In any case, NULL is not a valid listener, which is sort of > terrible, but it is how it is. You need to create an empty function > that does nothing. No, you don't. If one needs to shut up the compiler by adding NULLs, so be it. There is absolutely no need to pretend anything else, you are only shutting up the compiler. If anything ever happens to call that function pointer, you want to know about it, because it's a bug in the server most likely. So NULL will do just fine. A no-op function at best is just useless and at worst hides real bugs. I've never seen such warnings when building Weston, FWIW. This is all part of the interface extension mechanism. A client is allowed to be built against any revision of a protocol interface .xml definition, as far as Wayland is concerned. The client build itself may require a certain minimum version from the interface definitions. This is all completely normal as usual, similar to requiring certain minimal version of library headers to build against. Assume a client program has been written and compiles fine without warnings. Then, someone extends the protocol interface the client is using, adding new events. This means that the listener struct grows new fields at the end. This is perfectly safe and expected, and is intended to not require any changes to the client. Why is it safe? Because when the client binds to a global interface through wl_registry, it negotiates an interface version at runtime. The largest possible interface version is the minimum of three things: the interface version available in the headers/XML file, the interface version actually implemented in the client, and the interface version advertised by the server. If negotiation results in v1, then the server will never send events that were added in v2, just like Daniel explained. That is why it is not necessary to plug in valid functions or even NULLs into the listener struct fields a client will never need. I'm not sure, but I think we may even have version checks in libwayland such, that if a buggy server sent an event that is not defined by the negotiated interface version, it will be detected rather than calling an uninitialized or NULL function pointer in the listener. IMHO, in this particular case the compiler is warning without a reason. If Weston's build does not exhibit these warnings, I'd rather not apply this patch. If it does, I'd look into shutting up the compiler in a way that we don't have to patch all places every time an interface grows. Thanks, pq > On Tue, Jul 7, 2015 at 10:17 AM, Christopher Michael > <[email protected]> wrote: > > On 07/07/2015 01:15 PM, Jasper St. Pierre wrote: > >> > >> Shouldn't missing fields in structs be auto-initialized to 0 / NULL? I > >> thought that was part of the C specification. > >> > > > > I thought so also however when compiling some other code which was also > > creating a wl_output_listener, I uncovered the warnings about missing field > > initializers. When I looked/referenced the existing screenshooting code I > > noticed that they were missing from there also, so I just made a quick patch > > to address that. > > > > Cheers, > > Chris > > > > > >> On Tue, Jul 7, 2015 at 8:52 AM, Christopher Michael > >> <[email protected]> wrote: > >>> > >>> This patch adds missing placeholders for the wl_output listener > >>> functions 'done' and 'scale. Currently these placeholders are being > >>> set to NULL as the done and scale callbacks are not used in the > >>> screenshot client. > >>> > >>> Signed-off-by: Chris Michael <[email protected]> > >>> --- > >>> clients/screenshot.c | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/clients/screenshot.c b/clients/screenshot.c > >>> index f11e3ba..0d9b320 100644 > >>> --- a/clients/screenshot.c > >>> +++ b/clients/screenshot.c > >>> @@ -114,7 +114,9 @@ display_handle_mode(void *data, > >>> > >>> static const struct wl_output_listener output_listener = { > >>> display_handle_geometry, > >>> - display_handle_mode > >>> + display_handle_mode, > >>> + NULL, > >>> + NULL, > >>> }; > >>> > >>> static void > >>> -- _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
