GNOME Bugzilla – Bug 660531
Fix numerous implicit function declaration warnings
Last modified: 2011-12-17 22:15:35 UTC
When building vinagre-3.2.0 or git master (3.3.1), one encounters numerous scary implicit function declaration warnings: * vinagre/vinagre-options.c:140: warning: implicit declaration of function ‘vinagre_utils_show_many_errors’ * vinagre/vinagre-bookmarks-migration.c:213: warning: implicit declaration of function 'vinagre_utils_create_dir_for_file' * vinagre/vinagre-bookmarks-migration.c:221: warning: implicit declaration of function 'vinagre_dirs_get_user_data_dir' * vinagre/vinagre-bookmarks.c:78: warning: implicit declaration of function 'vinagre_dirs_get_user_data_dir' * vinagre/vinagre-bookmarks-ui.c:46: warning: implicit declaration of function 'vinagre_utils_get_builder' * vinagre/vinagre-bookmarks-ui.c:77: warning: implicit declaration of function 'vinagre_utils_show_error_dialog' * vinagre/vinagre-commands.c:138: warning: implicit declaration of function 'vinagre_utils_show_error_dialog' * vinagre/vinagre-commands.c:173: warning: implicit declaration of function 'vinagre_utils_show_many_errors' * vinagre/vinagre-commands.c:228: warning: implicit declaration of function 'vinagre_utils_set_widget_visible' * vinagre/vinagre-commands.c:315: warning: implicit declaration of function 'vinagre_utils_show_help' * vinagre/vinagre-commands.c:322: warning: implicit declaration of function 'vinagre_utils_show_help_about' * vinagre/vinagre-connect.c:71: warning: implicit declaration of function 'vinagre_dirs_get_user_data_dir' * vinagre/vinagre-connect.c:403: warning: implicit declaration of function 'vinagre_utils_get_builder' * vinagre/vinagre-connect.c:438: warning: implicit declaration of function 'vinagre_utils_show_help' * vinagre/vinagre-connect.c:494: warning: implicit declaration of function 'vinagre_utils_show_error_dialog' * vinagre/vinagre-connection.c:212: warning: implicit declaration of function 'vinagre_utils_parse_boolean' * vinagre/vinagre-notebook.c:463: warning: implicit declaration of function 'vinagre_utils_show_error_dialog' * vinagre/vinagre-reverse-vnc-listener.c:211: warning: implicit declaration of function 'vinagre_utils_show_error_dialog' * vinagre/vinagre-reverse-vnc-listener-dialog.c:147: warning: implicit declaration of function 'vinagre_utils_show_help' [...] * plugins/vnc/vinagre-vnc-tab.c:498: warning: implicit declaration of function 'vinagre_utils_request_credential' Most (but not all) of these are due to vinagre-utils and vinagre-dirs being converted from C to vala during the 3.1.x cycle. Since functions defined in vala are being used in C source, we will need to use valac to generate a header file that the C files can include. Note that due to the limitations of valac, there will have to be one header file (e.g. vinagre-utils.h) for both .vala source files.
Created attachment 197835 [details] [review] Fix implicit function declaration warnings Fix implicit declaration of vinagre_vnc_connection_set_depth_profile by including the plugins/vnc/vinagre-vnc-connection.h header. Fix gtk_widget_modify_bg implicit declaration warning by switching to the non-deprecated gtk_widget_override_background_color(). Fix *many* vinagre_utils_* and vinagre_dirs_* implicit declarations by generating a header (vinagre/vinagre-utils.h) from the vala source, and including it in half the .c files in the source tree. Since automake won't do it automatically, we have to write an explicit rule to generate the header from vala. Add the generated header to BUILT_SOURCES to ensure that it gets built before anything that might include it. And add it to noinst_vinagreh_headers to make sure that it's distributed with the tarball, so end users won't need valac to build vinagre.
Review of attachment 197835 [details] [review]: This single patch should be separated out into three separate patches, one for each paragraph in the commit message. ::: Makefile.am @@ -225,1 +229,5 @@ +BUILT_SOURCES = \ + $(built_headers) + +# manual rule to generate vinagre-utils.h, since automake won't automatically do it Rather than writing this rule manually, is it possible to build the Vala code first, generating the header as a side effect, and then build the remaining C code afterwards? In practice, the build process serialisation should be the same for both approaches.
(In reply to comment #2) > Rather than writing this rule manually, is it possible to build the Vala code > first, generating the header as a side effect, and then build the remaining C > code afterwards? In practice, the build process serialisation should be the > same for both approaches. Unfortunately, I could not come up with any non-manual way of generating the header. Automake's implicit rules for vala do not generate headers, and simply adding --header to AM_VALAFLAGS will not work because (1) --header needs the output filename as an argument, and (2) in any case you only want to generate the header from vinagre-utils.vala (since the header generated from vinagre-dirs.vala is a strict subset of it).
(In reply to comment #3) > Unfortunately, I could not come up with any non-manual way of generating the > header. How about just passing the relevant arguments to generate a single header file to valac via AM_VALAFLAGS? https://gitorious.org/vinagre/vinagre-elmarco/commit/d325b0bf51914e71ef14fc55a7cb98accb43fc8b > Automake's implicit rules for vala do not generate headers, and simply adding > --header to AM_VALAFLAGS will not work because (1) --header needs the output > filename as an argument, and (2) in any case you only want to generate the > header from vinagre-utils.vala (since the header generated from > vinagre-dirs.vala is a strict subset of it). It is intentional that Vala only generates a single header, and I would like to follow that intention, so just vinagre-vala.h (or similar) as in the commit linked to above, is fine. http://mail.gnome.org/archives/vala-list/2009-January/msg00086.html
Created attachment 197911 [details] [review] Fix implicit declaration of vinagre_vnc_connection_set_depth_profile() (In reply to comment #2) > This single patch should be separated out into three separate patches, one for > each paragraph in the commit message. OK, here is the first of three patches. It will need to applied before the others. Fix implicit declaration of vinagre_vnc_connection_set_depth_profile() in vinagre-tube-handler.c by including vinagre-vnc-connection.h
Created attachment 197912 [details] [review] Do not use deprecated gtk_widget_modify_bg() Second patch. Use gtk_widget_override_background_color() instead of deprecated gtk_widget_modify_bg(). Fixes an implicit function declaration warning when building with G_DISABLE_DEPRECATED.
Created attachment 197913 [details] [review] Fix vinagre_dirs_*, vinagre_utils_* implicit function declarations Third patch, the vinagre-vala.h one. With this patch, I wanted to meet the following goals: 1. building from git should work out of the box 2. building from tarball should work out of the box 3. do not break make dist 4. parallel-make safe (so need to ensure valac will not run on the same files multiple times simultaneously) 5. will continue to work if additional vala files are added to vinagre in the future. And satisfying these goals, particularly the last one, turned out to be a bit more difficult than I at first expected... --- Fix numerous vinagre_utils_* and vinagre_dirs_* implicit function declarations by generating a header (vinagre/vinagre-vala.h) from the vala source, and including it in half the .c files in the source tree. Add the vinagre-vala.h to dist_noinst_DATA to make sure it goes in the release tarballs, and to BUILT_SOURCES to ensure that it gets generated before the C sources that #include it are compiled. Unfortunately, automake does not support per-target VALAFLAGS. We cannot simply add a "--header vinagre/vinagre-vala.h" option to AM_VALAFLAGS or VALAFLAGS because if we ever use a .vala file in another target (such as one of the vinagre plugins), vinagre-vala.h will get clobbered. The only way to prevent valac from running multiple times and safely support parallel make without adding a ticking time bomb that could clobber vinagre-vala.h is to manually write a stamp target for the vala sources used in libvinagre, and manually pass a --header argument to valac in the rule for the stamp. Pick "libvinagre_vala.stamp" as the name for it to avoid potential collisions with automake's vala stamp naming conventions. And make sure to add the stamp to dist_noinst_DATA so that people who download the source tarball aren't forced to use vala to build vinagre. Also, add an explicit rule for generating vinagre-vala.h for those who do not have it and are building from git (otherwise, make fill fail due to vinagre-vala.h's presence in dist_noinst_DATA).
Comment on attachment 197911 [details] [review] Fix implicit declaration of vinagre_vnc_connection_set_depth_profile() Pushed to gnome-3-2 and master as commits 6c7de59a96135b05aea5cb02625524842614054c and 19ab67d6e16e017f18f3b2a6b6b1b556e055ca39.
Comment on attachment 197912 [details] [review] Do not use deprecated gtk_widget_modify_bg() Pushed to gnome-3-2 and master as commits eba994f4299a5096f0d35d8a7999d4c66d7789f1 and e1eaa709f5394c0121eb091fa93530d7419a45eb.
Comment on attachment 197913 [details] [review] Fix vinagre_dirs_*, vinagre_utils_* implicit function declarations Thanks for the long explanation in the commit message. Unfortunately, this breaks a distcheck, apparently because of the srcdir != builddir build. I get: rm: cannot remove `../libvinagre_vala.stamp': Permission denied make[4]: Entering directory `/home/david/documents/checkout/vinagre/vinagre-3.3.1' make[4]: Nothing to be done for `libvinagre_vala.stamp'. make[4]: Leaving directory `/home/david/documents/checkout/vinagre/vinagre-3.3.1' followed by: CC vinagre/vinagre-dirs.lo gcc: vinagre/vinagre-dirs.c: No such file or directory gcc: no input files make[3]: *** [vinagre/vinagre-dirs.lo] Error 1 when running a distcheck. I see that you copied the rules from the Vala stamp rules in the generated Makefile, so I cannot immediately see where the problem is, except that removing files in srcdir feels wrong. I will not have time to look at this again until Monday, so I can do another review then.
Created attachment 203710 [details] [review] Fix vinagre_dirs_*, vinagre_utils_* implicit function declarations (In reply to comment #10) > (From update of attachment 197913 [details] [review]) > Thanks for the long explanation in the commit message. Unfortunately, this > breaks a distcheck, apparently because of the srcdir != builddir build. OK, here is another attempt. I've added "-I$(top_srcdir)/vinagre" to CPPFLAGS, and now include the .vala files in dist_noinst_DATA since they are no longer in vinagre_vinagre_SOURCES. The patch applies to vinagre-3.3.x git head and passes distcheck. Hope that this version is acceptable :)
Comment on attachment 203710 [details] [review] Fix vinagre_dirs_*, vinagre_utils_* implicit function declarations Pushed the patch to master with a slightly-modified commit message as commit 75e6aac1b2c70654588b155fbb13be91ac9d3056. Thanks very much for the thorough investigation and explanation.