Check a return value from 'g_stat()'

Some Mixbus users (on Windows) have reported seeing ludicrously high figures for the amount of disk space that'll be recovered if they choose to clean up unused sources. I can't see anything obviously wrong in Ardour's code - except for one situation where we don't check a return value after calling 'g_stat()'.

On Windows, the relevant path should be (hopefully!) in UTF8 format and the first thing that g_stat() does is to convert it to UTF16. If that conversion fails for some reason, g_stat() will return an error status and statbuf will be uninitialized - but at the moment, we're not checking this. As an experiment, let's check the returned value and find out if these user reports go away.

Unfortunately, if it does fix the problem then we've got an even bigger problem - because somehow, a Windows user can create source files with invalid names which can't be later deleted!!

diff --git a/libs/ardour/session_state.cc b/libs/ardour/session_state.cc
index ed664f0dc87a5592bb15c4a6a3098ac5707db1ca..68ab485db702d0caf3e6c85e9fdb8e6cba32fd3c 100644 (file)
--- a/libs/ardour/session_state.cc
+++ b/libs/ardour/session_state.cc
@@ -3304,36 +3304,37 @@ Session::cleanup_sources (CleanupReport& rep)
 
                }
 
-               g_stat ((*x).c_str(), &statbuf);
+               if (0 == g_stat ((*x).c_str(), &statbuf)) {
 
-               if (::rename ((*x).c_str(), newpath.c_str()) != 0) {
-                       error << string_compose (_("cannot rename unused file source from %1 to %2 (%3)"), (*x), newpath, strerror (errno)) << endmsg;
-                       continue;
-               }
+                       if (::rename ((*x).c_str(), newpath.c_str()) != 0) {
+                               error << string_compose (_("cannot rename unused file source from %1 to %2 (%3)"), (*x), newpath, strerror (errno)) << endmsg;
+                               continue;
+                       }
 
-               /* see if there an easy to find peakfile for this file, and remove it.
-                */
+                       /* see if there an easy to find peakfile for this file, and remove it.
+                        */
 
-                string base = Glib::path_get_basename (*x);
-                base += "%A"; /* this is what we add for the channel suffix of all native files,
-                                 or for the first channel of embedded files. it will miss
-                                 some peakfiles for other channels
-                              */
-               string peakpath = construct_peak_filepath (base);
-
-               if (Glib::file_test (peakpath.c_str(), Glib::FILE_TEST_EXISTS)) {
-                       if (::g_unlink (peakpath.c_str()) != 0) {
-                               error << string_compose (_("cannot remove peakfile %1 for %2 (%3)"),
-                                                         peakpath, _path, strerror (errno))
-                                     << endmsg;
-                               /* try to back out */
-                               ::rename (newpath.c_str(), _path.c_str());
-                               goto out;
+                                       string base = Glib::path_get_basename (*x);
+                                       base += "%A"; /* this is what we add for the channel suffix of all native files,
+                                                                        or for the first channel of embedded files. it will miss
+                                                                        some peakfiles for other channels
+                                                                 */
+                       string peakpath = construct_peak_filepath (base);
+
+                       if (Glib::file_test (peakpath.c_str(), Glib::FILE_TEST_EXISTS)) {
+                               if (::g_unlink (peakpath.c_str()) != 0) {
+                                       error << string_compose (_("cannot remove peakfile %1 for %2 (%3)"),
+                                                                                                                        peakpath, _path, strerror (errno))
+                                                 << endmsg;
+                                       /* try to back out */
+                                       ::rename (newpath.c_str(), _path.c_str());
+                                       goto out;
+                               }
                        }
-               }
 
-               rep.paths.push_back (*x);
-               rep.space += statbuf.st_size;
+                       rep.paths.push_back (*x);
+                       rep.space += statbuf.st_size;
+               }
        }
 
        /* dump the history list */