After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759200 - Fix -Werror build for clang
Fix -Werror build for clang
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Build
git master
Other FreeBSD
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-12-08 18:22 UTC by Ting-Wei Lan
Modified: 2015-12-15 14:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: Fix build with clang -Werror and -Wwrite-strings (18.38 KB, patch)
2015-12-08 18:24 UTC, Ting-Wei Lan
committed Details | Review
build: Fix build with clang -Werror and -Wwrite-strings (771 bytes, patch)
2015-12-14 19:22 UTC, Ting-Wei Lan
committed Details | Review

Description Ting-Wei Lan 2015-12-08 18:22:54 UTC
Epiphany uses -Wwrite-strings, which changes the type of string literals to 'const char[]'. This means any attempt to use a string literal as 'char*' causes build failure.

I will attach a patch that fixes the build. I think it needs some testing and review before it can be pushed.
Comment 1 Ting-Wei Lan 2015-12-08 18:24:52 UTC
Created attachment 316968 [details] [review]
build: Fix build with clang -Werror and -Wwrite-strings

String literals are 'const char[]' when -Wwrite-strings is used.
Comment 2 Michael Catanzaro 2015-12-08 19:29:56 UTC
Review of attachment 316968 [details] [review]:

Thanks for fixing this. Please, address the comments before pushing.

::: embed/ephy-download.c
@@ -164,3 @@
       action = EPHY_DOWNLOAD_ACTION_OPEN;
-
-    g_free (content_type);

Oh my, this looks like a potentially-serious bug. Please, commit this in a separate patch, and to gnome-3-18 as well.

::: embed/ephy-embed-prefs.c
@@ +428,3 @@
     /* Sync with Epiphany values */
     webkit_pref_callback_font_size (ephy_settings, EPHY_PREFS_WEB_SERIF_FONT,
+                                    (void*)"default-font-size");

Use gpointer for these. Please fix everywhere.

::: embed/web-extension/ephy-uri-tester.c
@@ +443,3 @@
 ephy_uri_tester_compile_regexp (EphyUriTester *tester,
                                 GString   *gpatt,
+                                const char*opts,

Add spaces, please; there should be at least one blank space. I only mention this once, but please fix everywhere.

@@ +520,3 @@
             g_free (patt);
         if (data[1])
+            g_free ((char*)opts);

(char *)opts. Please fix everywhere.

::: lib/ephy-profile-utils.c
@@ +129,3 @@
     argv[0] = ABS_TOP_BUILD_DIR"/lib/"EPHY_PROFILE_MIGRATOR;
 
+  ret = g_spawn_sync (NULL, (char**)argv, envp, G_SPAWN_SEARCH_PATH,

(char **)

::: src/bookmarks/ephy-bookmark-properties.c
@@ -40,3 @@
 #include <string.h>
 
-static const GtkTargetEntry dest_drag_types[] = {

Go ahead and commit this separately too, please.
Comment 3 Ting-Wei Lan 2015-12-11 10:04:08 UTC
Fix for decide_action_from_mime was pushed to master as commit 1387537 and pushed to gnome-3-18 as commit 3d1a0d0.

Removal of dest_drag_types was pushed to master as commit 4342778.

Other modifications were pushed to master as commit 7c7d30f.
Comment 4 Michael Catanzaro 2015-12-11 10:18:37 UTC
Aahhh, I see now, so get_content_type in 3.18 returns a string that needs to be freed, but in master it should not be freed. So I was wrong to suggest a backport, and have reverted your commit on the gnome-3-18 branch, but not on master. This seems good otherwise. Thanks!
Comment 5 Ting-Wei Lan 2015-12-14 19:22:24 UTC
Created attachment 317391 [details] [review]
build: Fix build with clang -Werror and -Wwrite-strings

Current master branch still doesn't build with -Werror because new commits
cause a new error. This patch should fix it.
Comment 6 Michael Catanzaro 2015-12-15 13:14:08 UTC
Review of attachment 317391 [details] [review]:

Thanks. Wish GCC had this warning....

By the way, if you are using an automated build system, I recommend passing --disable-Werror to every configure script, like jhbuild does.
Comment 7 Ting-Wei Lan 2015-12-15 14:10:22 UTC
Attachment 317391 [details] pushed as 06152d5 - build: Fix build with clang -Werror and -Wwrite-strings