IgnorantGuru / spacefm

SpaceFM File Manager
http://ignorantguru.github.com/spacefm/
GNU General Public License v3.0
489 stars 73 forks source link

spacefm 0.9.3 get FTBFS when -Wformat-security is enabled #416

Closed mati75 closed 10 years ago

mati75 commented 10 years ago

Hello, the lastest version spacefm get FTBFS on Debian when -Wformat-security is enabled, I contributing package with this patch fixed the problem:

--- a/src/vfs/vfs-app-desktop.c
+++ b/src/vfs/vfs-app-desktop.c
@@ -584,7 +584,7 @@ gboolean vfs_app_desktop_open_files( Gdk
     }

     cmd = g_strdup_printf( "%s\n\n%s", _("Command not found"), app->file_name );
-    g_set_error( err, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, cmd );
+    g_set_error( err, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, "%s", cmd );
     g_free( cmd );
     return FALSE;
 }

Please add it to sources.

mtasaka commented 10 years ago

Also fails to build on Fedora rawhide (with -Werror=format-security). And I think the following is better;

diff --git a/src/vfs/vfs-app-desktop.c b/src/vfs/vfs-app-desktop.c
index cec43d8..161f4f4 100644
--- a/src/vfs/vfs-app-desktop.c
+++ b/src/vfs/vfs-app-desktop.c
@@ -583,8 +583,12 @@ gboolean vfs_app_desktop_open_files( GdkScreen* screen,
         return TRUE;
     }

-    cmd = g_strdup_printf( "%s\n\n%s", _("Command not found"), app->file_name );
-    g_set_error( err, G_SPAWN_ERROR, G_SPAWN_ERROR_FAILED, cmd );
-    g_free( cmd );
+    g_set_error( err,
+                 G_SPAWN_ERROR,
+                 G_SPAWN_ERROR_FAILED,
+                 "%s\n\n%s",
+                 _("Command not found"),
+                 app->file_name
+                  );
     return FALSE;
 }
-- 
1.8.5.3
IgnorantGuru commented 10 years ago

Thanks. This commit also adds these default make options in configure.ac:

CPPFLAGS="$CPPFLAGS -Wno-deprecated-declarations -Wformat -Wformat-security -Wreturn-type -Wunused-value"

Hopefully that won't break any compilers? Or I can trim it back.

mtasaka commented 10 years ago

Thank you. By the way, out of curiosity I looked at commit 9618c6205dd567e353210c68c7927e790ee0c660 , and for src/ptk/ptk-clipboard.c:

--- a/src/ptk/ptk-clipboard.c
+++ b/src/ptk/ptk-clipboard.c
@@ -618,7 +618,8 @@ GList* ptk_clipboard_get_file_paths( const char* cwd, gboolean* is_cut,
                 files = g_list_prepend( files, file_path );
             }
             else
-                *missing_targets++;
+                // no *missing_targets++ here to avoid -Wunused-value compiler warning
+                *missing_targets = *missing_targets + 1;
         }
         ++puri;
     }

So this is meant to be

(*missing_targets)++;

instead of *missing_targets++; ? (I am just asking because I just saw it...)

IgnorantGuru commented 10 years ago

Okay thanks, I was wondering how to get the compiler happy with that. But it's good for now. I didn't write that code - I think it should have been (*missing_targets)++ originally.