imageicon: clean up unnecessary file existance check introduced in rev 12263. Could theoretically lead to uninitialized value use (of "info" struct)

same for filedialogimpl-gtkmm

(bzr r13240)

2 files changed, 36 insertions, 50 deletions

diff --git a/src/ui/dialog/filedialogimpl-gtkmm.cpp b/src/ui/dialog/filedialogimpl-gtkmm.cpp
index fdc9cecc3..60b437575 100644
--- a/src/ui/dialog/filedialogimpl-gtkmm.cpp
+++ b/src/ui/dialog/filedialogimpl-gtkmm.cpp
@@ -514,10 +514,11 @@ void SVGPreview::showTooLarge(long fileLength)
 bool SVGPreview::set(Glib::ustring &fileName, int dialogType)
 {
 
-    if (!Glib::file_test(fileName, Glib::FILE_TEST_EXISTS))
+    if (!Glib::file_test(fileName, Glib::FILE_TEST_EXISTS)) {
+        showNoPreview();
         return false;
+    }
 
-    //g_message("fname:%s", fileName.c_str());
 
     if (Glib::file_test(fileName, Glib::FILE_TEST_IS_DIR)) {
         showNoPreview();
@@ -525,24 +526,22 @@ bool SVGPreview::set(Glib::ustring &fileName, int dialogType)
     }
 
     if (Glib::file_test(fileName, Glib::FILE_TEST_IS_REGULAR))
-        {
+    {
         Glib::ustring fileNameUtf8 = Glib::filename_to_utf8(fileName);
-        gchar *fName = const_cast<gchar *>(fileNameUtf8.c_str());
+        gchar *fName = const_cast<gchar *>(fileNameUtf8.c_str()); // const-cast probably not necessary? (not necessary on Windows version of stat())
         struct stat info;
-        if (g_file_test (fName, G_FILE_TEST_EXISTS) && g_stat(fName, &info))
-            {
-            g_warning("SVGPreview::set() : %s : %s",
-                           fName, strerror(errno));
-            return FALSE;
-            }
-        long fileLen = info.st_size;
-        if (fileLen > 0xA00000L)
-            {
+        if (g_stat(fName, &info)) // stat returns 0 upon success
+        {
+            g_warning("SVGPreview::set() : %s : %s", fName, strerror(errno));
+            return false;
+        }
+        if (info.st_size > 0xA00000L)
+        {
             showingNoPreview = false;
-            showTooLarge(fileLen);
-            return FALSE;
-            }
+            showTooLarge(info.st_size);
+            return false;
         }
+    }
 
     Glib::ustring svg = ".svg";
     Glib::ustring svgz = ".svgz";
diff --git a/src/ui/widget/imageicon.cpp b/src/ui/widget/imageicon.cpp
index cf41a16a4..22abd04ba 100644
--- a/src/ui/widget/imageicon.cpp
+++ b/src/ui/widget/imageicon.cpp
@@ -369,60 +369,47 @@ isValidImageIconFile(const Glib::ustring &fileName)
     return false;
 }
 
+/// \fixme This function is almost an exact duplicate of SVGPreview::set() in ui/dialog/filedialogimpl-gtkmm.cpp.
 bool ImageIcon::show(const Glib::ustring &fileName)
 {
-
-    if (!Glib::file_test(fileName, Glib::FILE_TEST_EXISTS))
+    if (!Glib::file_test(fileName, Glib::FILE_TEST_EXISTS)) {
+        showBrokenImage("File does not exist");
         return false;
+    }
 
-    gchar *fName = const_cast<gchar *>(fileName.c_str());
-    //g_message("fname:%s\n", fName);
-
-
-    if (Glib::file_test(fileName, Glib::FILE_TEST_IS_REGULAR))
-        {
+    if (Glib::file_test(fileName, Glib::FILE_TEST_IS_REGULAR)) {
+        gchar *fName = const_cast<gchar *>(fileName.c_str()); // this const-cast seems not necessary, was it put there because of older sys/stat.h version?
         struct stat info;
-        if (g_file_test (fName, G_FILE_TEST_EXISTS) && stat(fName, &info))
-            {
-            Glib::ustring err = "cannot get file info";
-            showBrokenImage(err);
+        if (stat(fName, &info)) // stat returns 0 upon success
+        {
+            showBrokenImage("Cannot get file info");
             return false;
-            }
-        long fileLen = info.st_size;
-        if (fileLen > 0x150000L)
-            {
-            Glib::ustring err = "File too large";
-            showBrokenImage(err);
+        }
+        if (info.st_size > 0x150000L) {
+            showBrokenImage("File too large");
             return false;
-            }
         }
+    }
 
     Glib::ustring svg = ".svg";
     Glib::ustring svgz = ".svgz";
 
-    if (hasSuffix(fileName, svg) || hasSuffix(fileName, svgz)   )
-        {
-        if (!showSvgFile(fileName))
-            {
+    if (hasSuffix(fileName, svg) || hasSuffix(fileName, svgz)) {
+        if (!showSvgFile(fileName)) {
             showBrokenImage(bitmapError);
             return false;
-            }
-        return true;
         }
-    else if (isValidImageIconFile(fileName))
-        {
-        if (!showBitmap(fileName))
-            {
+        return true;
+    } else if (isValidImageIconFile(fileName)) {
+        if (!showBitmap(fileName)) {
             showBrokenImage(bitmapError);
             return false;
-            }
-        return true;
         }
-    else
-        {
+        return true;
+    } else {
         showBrokenImage("unsupported file type");
         return false;
-        }
+    }
 }