Merge pull request #710 from jphickey/fix-708-chmod-err-handling

Fix #708, chmod error handling

src/os/portable/os-impl-posix-files.c

@@ -203,6 +203,7 @@ int32 OS_FileChmod_Impl(const char *local_path, uint32 access)
     mode_t      writebits;
     struct stat st;
     int         fd;
+    int32       status;
 
     /* Open file to avoid filename race potential */
     fd = open(local_path, O_RDONLY, 0);
@@ -211,10 +212,16 @@ int32 OS_FileChmod_Impl(const char *local_path, uint32 access)
         fd = open(local_path, O_WRONLY, 0);
         if (fd < 0)
         {
+            OS_DEBUG("open(%s): %s (%d)\n", local_path, strerror(errno), errno);
             return OS_ERROR;
-        }        
+        }
     }
 
+    /*
+     * NOTE: After this point, execution must proceed to the end of this routine
+     * so that the "fd" opened above can be properly closed.
+     */
+
     /*
      * In order to preserve any OTHER mode bits,
      * first stat() the file and then modify the st_mode
@@ -226,58 +233,81 @@ int32 OS_FileChmod_Impl(const char *local_path, uint32 access)
      */
     if (fstat(fd, &st) < 0)
     {
-        return OS_ERROR;
+        OS_DEBUG("fstat(%s): %s (%d)\n", local_path, strerror(errno), errno);
+        status = OS_ERROR;
     }
-
-    /* always check world bits */
-    readbits  = S_IROTH;
-    writebits = S_IWOTH;
-
-    if (OS_IMPL_SELF_EUID == st.st_uid)
+    else
     {
-        /* we own the file so use user bits */
-        readbits |= S_IRUSR;
-        writebits |= S_IWUSR;
-    }
+        /* always check world bits */
+        readbits  = S_IROTH;
+        writebits = S_IWOTH;
 
-    if (OS_IMPL_SELF_EGID == st.st_gid)
-    {
-        /* our group owns the file so use group bits */
-        readbits |= S_IRGRP;
-        writebits |= S_IWGRP;
-    }
+        if (OS_IMPL_SELF_EUID == st.st_uid)
+        {
+            /* we own the file so use user bits */
+            readbits |= S_IRUSR;
+            writebits |= S_IWUSR;
+        }
 
-    if (access == OS_WRITE_ONLY || access == OS_READ_WRITE)
-    {
-        /* set all "write" mode bits */
-        st.st_mode |= writebits;
-    }
-    else
-    {
-        /* clear all "write" mode bits */
-        st.st_mode &= ~writebits;
-    }
+        if (OS_IMPL_SELF_EGID == st.st_gid)
+        {
+            /* our group owns the file so use group bits */
+            readbits |= S_IRGRP;
+            writebits |= S_IWGRP;
+        }
 
-    if (access == OS_READ_ONLY || access == OS_READ_WRITE)
-    {
-        /* set all "read" mode bits */
-        st.st_mode |= readbits;
-    }
-    else
-    {
-        /* clear all "read" mode bits */
-        st.st_mode &= ~readbits;
-    }
+        if (access == OS_WRITE_ONLY || access == OS_READ_WRITE)
+        {
+            /* set all "write" mode bits */
+            st.st_mode |= writebits;
+        }
+        else
+        {
+            /* clear all "write" mode bits */
+            st.st_mode &= ~writebits;
+        }
 
-    /* finally, write the modified mode back to the file */
-    if (fchmod(fd, st.st_mode) < 0)
-    {
-        return OS_ERROR;
+        if (access == OS_READ_ONLY || access == OS_READ_WRITE)
+        {
+            /* set all "read" mode bits */
+            st.st_mode |= readbits;
+        }
+        else
+        {
+            /* clear all "read" mode bits */
+            st.st_mode &= ~readbits;
+        }
+
+        /* finally, write the modified mode back to the file */
+        if (fchmod(fd, st.st_mode) < 0)
+        {
+            /*
+             * These particular errnos generally indicate that the
+             * underlying filesystem does not support chmod()
+             *
+             * This is often the case for FAT / DOSFS filesystems
+             * which do not have UNIX-style permissions, or (in the
+             * case of EROFS) if the filesystem is mounted read-only.
+             */
+            if (errno == ENOTSUP || errno == ENOSYS || errno == EROFS)
+            {
+                status = OS_ERR_NOT_IMPLEMENTED;
+            }
+            else
+            {
+                OS_DEBUG("fchmod(%s): %s (%d)\n", local_path, strerror(errno), errno);
+                status = OS_ERROR;
+            }
+        }
+        else
+        {
+            status = OS_SUCCESS;
+        }
     }
 
     close(fd);
 
-    return OS_SUCCESS;
+    return status;
 
 } /* end OS_FileChmod_Impl */
 

src/unit-test-coverage/portable/src/coveragetest-posix-files.c

@@ -35,6 +35,7 @@
 #include <OCS_unistd.h>
 #include <OCS_fcntl.h>
 #include <OCS_stat.h>
+#include <OCS_errno.h>
 
 void Test_OS_FileOpen_Impl(void)
 {
@@ -114,6 +115,10 @@ void Test_OS_FileChmod_Impl(void)
     /* failure mode 2 (fchmod) */
     UT_SetDefaultReturnValue(UT_KEY(OCS_fchmod), -1);
     OSAPI_TEST_FUNCTION_RC(OS_FileChmod_Impl, ("local", OS_READ_WRITE), OS_ERROR);
+
+    /* non implemented error, e.g. such as DOS Filesystem with no perms  */
+    OCS_errno = OCS_ENOTSUP;
+    OSAPI_TEST_FUNCTION_RC(OS_FileChmod_Impl, ("local", OS_READ_WRITE), OS_ERR_NOT_IMPLEMENTED);
     UT_ClearForceFail(UT_KEY(OCS_fchmod));
 
     /* all permission bits with uid/gid match */

src/unit-test-coverage/ut-stubs/inc/OCS_errno.h

@@ -34,6 +34,9 @@
 #define OCS_EMSGSIZE  0x1804
 #define OCS_ETIMEDOUT 0x1805
 #define OCS_ESPIPE    0x1806
+#define OCS_ENOTSUP   0x1807
+#define OCS_ENOSYS    0x1808
+#define OCS_EROFS     0x1809
 
 /* ----------------------------------------- */
 /* types normally defined in errno.h */

src/unit-test-coverage/ut-stubs/override_inc/errno.h

@@ -33,6 +33,9 @@
 #define EMSGSIZE  OCS_EMSGSIZE
 #define ETIMEDOUT OCS_ETIMEDOUT
 #define ESPIPE    OCS_ESPIPE
+#define ENOTSUP   OCS_ENOTSUP
+#define ENOSYS    OCS_ENOSYS
+#define EROFS     OCS_EROFS
 
 #define errno OCS_errno