Fix issue 360: Cannot save gif files

* SaveFile and SaveFileAs commands ask for removing read-only attribute
* Fixed base::open_file_descriptor_with_exception() to create new files
* Added internal base::Win32Exception
* Added unit tests for the creation of file descriptors
* Added base::delete_file, has_readonly_attr, remove_readonly_attr functions

src/app/commands/cmd_save_file.cpp

@@ -101,6 +101,22 @@ static void save_document_in_background(Document* document, bool mark_as_saved)
 
 //////////////////////////////////////////////////////////////////////
 
+static bool confirm_readonly(const base::string& filename)
+{
+  if (!base::has_readonly_attr(filename))
+    return true;
+
+  int ret = ui::Alert::show("Warning<<The file is read-only, do you really want to overwrite it?<<%s||&Yes||&No",
+    base::get_file_name(filename).c_str());
+
+  if (ret == 1) {
+    base::remove_readonly_attr(filename);
+    return true;
+  }
+  else
+    return false;
+}
+
 static void save_as_dialog(const ContextReader& reader, const char* dlg_title, bool mark_as_saved)
 {
   const Document* document = reader.document();
@@ -119,17 +135,26 @@ static void save_as_dialog(const ContextReader& reader, const char* dlg_title, b
 
     filename = newfilename;
 
-    if (base::file_exists(filename.c_str())) {
+    // Ask if the user wants overwrite the existent file.
-      // Ask if the user wants overwrite the existent file?
+    if (base::file_exists(filename)) {
-      ret = ui::Alert::show("Warning<<File exists, overwrite it?<<%s||&Yes||&No||&Cancel",
+      ret = ui::Alert::show("Warning<<The file already exists, overwrite it?<<%s||&Yes||&No||&Cancel",
-                            base::get_file_name(filename).c_str());
+        base::get_file_name(filename).c_str());
+
+      // Check for read-only attribute.
+      if (ret == 1) {
+        if (!confirm_readonly(filename))
+          ret = 2;              // Select file again.
+        else
+          break;
+      }
     }
     else
       break;
 
     // "yes": we must continue with the operation...
-    if (ret == 1)
+    if (ret == 1) {
       break;
+    }
     // "cancel" or <esc> per example: we back doing nothing
     else if (ret != 2)
       return;
@@ -188,6 +213,9 @@ void SaveFileCommand::onExecute(Context* context)
     ContextWriter writer(reader);
     Document* documentWriter = writer.document();
 
+    if (!confirm_readonly(documentWriter->getFilename()))
+      return;
+
     save_document_in_background(documentWriter, true);
     update_screen_for_document(documentWriter);
   }

src/base/CMakeLists.txt

@@ -15,7 +15,7 @@ if(HAVE_SCHED_YIELD)
   add_definitions(-DHAVE_SCHED_YIELD)
 endif()
 
-add_library(base-lib
+set(BASE_SOURCES
   cfile.cpp
   chrono.cpp
   convert_to.cpp
@@ -40,3 +40,10 @@ add_library(base-lib
   thread.cpp
   trim_string.cpp
   version.cpp)
+
+if(WIN32)
+  set(BASE_SOURCES ${BASE_SOURCES}
+    win32_exception.cpp)
+endif()
+
+add_library(base-lib ${BASE_SOURCES})

src/base/file_handle.cpp

@@ -16,6 +16,7 @@
 
 #ifdef WIN32
 #include <windows.h>
+#include <sys/stat.h>
 #else
 #include <fcntl.h>
 #endif
@@ -51,14 +52,14 @@ int open_file_descriptor_with_exception(const string& filename, const string& mo
 {
   int flags = 0;
   if (mode.find('r') != string::npos) flags |= O_RDONLY;
-  if (mode.find('w') != string::npos) flags |= O_WRONLY | O_TRUNC;
+  if (mode.find('w') != string::npos) flags |= O_RDWR | O_CREAT | O_TRUNC;
   if (mode.find('b') != string::npos) flags |= O_BINARY;
 
   int fd;
 #ifdef WIN32
-  fd = _wopen(from_utf8(filename).c_str(), flags);
+  fd = _wopen(from_utf8(filename).c_str(), flags, _S_IREAD | _S_IWRITE);
 #else
-  fd = open(filename.c_str(), flags);
+  fd = open(filename.c_str(), flags, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);
 #endif
 
   if (fd == -1)

src/base/file_handle_unittest.cpp

@@ -0,0 +1,66 @@
+// Aseprite Base Library
+// Copyright (c) 2001-2013 David Capello
+//
+// This source file is distributed under MIT license,
+// please read LICENSE.txt for more information.
+
+#include <gtest/gtest.h>
+
+#include "base/file_handle.h"
+#include "base/fs.h"
+#include "base/path.h"
+
+#include <vector>
+
+#ifdef WIN32
+#include <windows.h>
+#else
+#include <fcntl.h>
+#endif
+
+using namespace base;
+
+#pragma warning (disable: 4996)
+
+TEST(FileHandle, Descriptors)
+{
+  const char* fn = "test.txt";
+
+  // Delete the file if it exists.
+  ASSERT_NO_THROW({
+      if (file_exists(fn))
+        delete_file(fn);
+    });
+
+  // Create file.
+  ASSERT_NO_THROW({
+      int fd = open_file_descriptor_with_exception(fn, "wb");
+      close(fd);
+    });
+
+  // Truncate file.
+  ASSERT_NO_THROW({
+      int fd = open_file_descriptor_with_exception(fn, "wb");
+      EXPECT_EQ(6, write(fd, "hello", 6));
+      close(fd);
+    });
+
+  // Read.
+  ASSERT_NO_THROW({
+      int fd = open_file_descriptor_with_exception(fn, "rb");
+      std::vector<char> buf(6);
+      EXPECT_EQ(6, read(fd, &buf[0], 6));
+      EXPECT_EQ("hello", std::string(&buf[0]));
+      close(fd);
+    });
+
+  ASSERT_NO_THROW({
+      delete_file(fn);
+    });
+}
+
+int main(int argc, char** argv)
+{
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

src/base/fs.h

@@ -14,6 +14,11 @@ namespace base {
   bool file_exists(const string& path);
   bool directory_exists(const string& path);
 
+  void delete_file(const string& path);
+
+  bool has_readonly_attr(const string& path);
+  void remove_readonly_attr(const string& path);
+
   void make_directory(const string& path);
   void remove_directory(const string& path);
 

src/base/fs_unix.h

@@ -42,6 +42,32 @@ void make_directory(const string& path)
   }
 }
 
+void delete_file(const string& path)
+{
+  int result = unlink(path.c_str());
+  if (result != 0)
+    // TODO add errno into the exception
+    throw std::runtime_error("Error deleting file");
+}
+
+bool has_readonly_attr(const string& path)
+{
+  struct stat sts;
+  return (stat(path.c_str(), &sts) == 0 && ((sts.st_mode & S_IWUSR) == 0));
+}
+
+void remove_readonly_attr(const string& path)
+{
+  struct stat sts;
+  int result = stat(path.c_str(), &sts);
+  if (result == 0) {
+    result = chmod(path.c_str(), sts.st_mode | S_IWUSR);
+    if (result != 0)
+      // TODO add errno into the exception
+      throw std::runtime_error("Error removing read-only attribute");
+  }
+}
+
 void remove_directory(const string& path)
 {
   int result = rmdir(path.c_str());

src/base/fs_win32.h

@@ -4,10 +4,11 @@
 // This source file is distributed under MIT license,
 // please read LICENSE.txt for more information.
 
-#include <windows.h>
 #include <stdexcept>
+#include <windows.h>
 
 #include "base/string.h"
+#include "base/win32_exception.h"
 
 namespace base {
 
@@ -29,22 +30,41 @@ bool directory_exists(const string& path)
           ((attr & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY));
 }
 
+void delete_file(const string& path)
+{
+  BOOL result = ::DeleteFile(from_utf8(path).c_str());
+  if (result == 0)
+    throw Win32Exception("Error deleting file");
+}
+
+bool has_readonly_attr(const string& path)
+{
+  std::wstring fn = from_utf8(path);
+  DWORD attr = ::GetFileAttributes(fn.c_str());
+  return ((attr & FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY);
+}
+
+void remove_readonly_attr(const string& path)
+{
+  std::wstring fn = from_utf8(path);
+  DWORD attr = ::GetFileAttributes(fn.c_str());
+  if ((attr & FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY)
+    ::SetFileAttributes(fn.c_str(), attr & ~FILE_ATTRIBUTE_READONLY);
+}
+
 void make_directory(const string& path)
 {
   BOOL result = ::CreateDirectory(from_utf8(path).c_str(), NULL);
-  if (result == 0) {
+  if (result == 0)
-    // TODO add GetLastError() value into the exception
+    throw Win32Exception("Error creating directory");
-    throw std::runtime_error("Error creating directory");
-  }
 }
 
+
 void remove_directory(const string& path)
 {
   BOOL result = ::RemoveDirectory(from_utf8(path).c_str());
-  if (result == 0) {
+  if (result == 0)
-    // TODO add GetLastError() value into the exception
+    throw Win32Exception("Error removing directory");
-    throw std::runtime_error("Error removing directory");
-  }
 }
 
 string get_app_path()

src/base/win32_exception.cpp

@@ -0,0 +1,43 @@
+// Aseprite Base Library
+// Copyright (c) 2001-2013 David Capello
+//
+// This source file is distributed under MIT license,
+// please read LICENSE.txt for more information.
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "base/win32_exception.h"
+
+#include "base/string.h"
+
+#include <windows.h>
+
+namespace base {
+
+Win32Exception::Win32Exception(const std::string& msg) throw()
+  : Exception()
+{
+  DWORD errcode = GetLastError();
+  LPVOID buf;
+
+  FormatMessage(
+    FORMAT_MESSAGE_ALLOCATE_BUFFER | // TODO Try to use a TLS buffer
+    FORMAT_MESSAGE_FROM_SYSTEM |
+    FORMAT_MESSAGE_IGNORE_INSERTS,
+    NULL,
+    errcode,
+    MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+    (LPWSTR)&buf,
+    0, NULL);
+
+  setMessage((msg + "\n" + to_utf8((LPWSTR)buf)).c_str());
+  LocalFree(buf);
+}
+
+Win32Exception::~Win32Exception() throw()
+{
+}
+
+} // namespace base

src/base/win32_exception.h

@@ -0,0 +1,22 @@
+// Aseprite Base Library
+// Copyright (c) 2001-2013 David Capello
+//
+// This source file is distributed under MIT license,
+// please read LICENSE.txt for more information.
+
+#ifndef BASE_WIN32_EXCEPTION_H_INCLUDED
+#define BASE_WIN32_EXCEPTION_H_INCLUDED
+
+#include "exception.h"
+
+namespace base {
+
+  class Win32Exception : public Exception {
+  public:
+    Win32Exception(const std::string& msg) throw();
+    virtual ~Win32Exception() throw();
+  };
+
+}
+
+#endif