Merge pull request #617 from jphickey/fix-556-deprecate-open-creat

Fix #556, Deprecate OS_open and OS_creat
nasa · Oct 13, 2020 · 3a76641 · 3a76641
2 parents 2b6f2d1 + 1776749
commit 3a76641
Show file tree

Hide file tree

Showing 8 changed files with 179 additions and 179 deletions.
diff --git a/src/os/inc/osapi-os-filesys.h b/src/os/inc/osapi-os-filesys.h
@@ -178,6 +178,8 @@ typedef enum
  * @{
  */
 
+#ifndef OSAL_OMIT_DEPRECATED
+
 /*-------------------------------------------------------------------------------------*/
 /**
  * @brief Creates a file specified by path
@@ -199,6 +201,9 @@ typedef enum
  * @retval #OS_FS_ERR_NAME_TOO_LONG if the name of the file is too long
  * @retval #OS_ERROR if permissions are unknown or OS call fails
  * @retval #OS_ERR_NO_FREE_IDS if there are no free file descriptors left
+ *
+ * @deprecated Replaced by OS_OpenCreate() with flags set to
+ *             OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE.
  */
 int32           OS_creat  (const char *path, int32  access);
 
@@ -225,9 +230,14 @@ int32           OS_creat  (const char *path, int32  access);
  * @retval #OS_FS_ERR_NAME_TOO_LONG if the name of the file is too long
  * @retval #OS_ERROR if permissions are unknown or OS call fails
  * @retval #OS_ERR_NO_FREE_IDS if there are no free file descriptors left
+ *
+ * @deprecated Replaced by OS_OpenCreate() with flags set to
+ *             OS_FILE_FLAG_NONE.
  */
 int32           OS_open   (const char *path,  int32 access,  uint32 mode);
 
+#endif
+
 /*-------------------------------------------------------------------------------------*/
 /**
  * @brief Open or create a file

diff --git a/src/os/shared/src/osapi-file.c b/src/os/shared/src/osapi-file.c
@@ -99,6 +99,21 @@ int32 OS_OpenCreate(osal_id_t *filedes, const char *path, int32 flags, int32 acc
    OS_common_record_t *record;
    char   local_path[OS_MAX_LOCAL_PATH_LEN];
 
+   if (filedes == NULL)
+   {
+       return OS_INVALID_POINTER;
+   }
+
+   /*
+   ** Check for a valid access mode
+   */
+   if (access != OS_WRITE_ONLY &&
+           access != OS_READ_ONLY &&
+           access != OS_READ_WRITE)
+   {
+       return OS_ERROR;
+   }
+
    /*
     * Translate the path
     */
@@ -127,6 +142,12 @@ int32 OS_OpenCreate(osal_id_t *filedes, const char *path, int32 flags, int32 acc
 } /* end OS_OpenCreate */
 
 
+/*
+ * The OS_open and OS_creat functions are deprecated, replaced by
+ * the generic OS_OpenCreate above
+ */
+#ifndef OSAL_OMIT_DEPRECATED
+
 /*----------------------------------------------------------------
  *
  * Function: OS_creat
@@ -205,7 +226,7 @@ int32 OS_open   (const char *path,  int32 access,  uint32  mode)
    return return_code;
 } /* end OS_open */
 
-
+#endif
 
 /*----------------------------------------------------------------
  *

diff --git a/src/tests/file-api-test/file-api-test.c b/src/tests/file-api-test/file-api-test.c
@@ -147,22 +147,16 @@ void TestCreatRemove(void)
     }
 
     /* create a file with short name */
-    status = OS_creat(filename,OS_READ_WRITE);
-    UtAssert_True(status >= 0, "fd after creat short name length file = %d",(int)status);
-
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
+    status = OS_OpenCreate(&fd, filename, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
+    UtAssert_True(status == OS_SUCCESS, "fd after creat short name length file = %d",(int)status);
 
     /* close the first file */
     status = OS_close(fd);
     UtAssert_True(status == OS_SUCCESS, "status after close short name length file = %d",(int)status);
 
     /* create a file with max name size */
-    status = OS_creat(maxfilename,OS_READ_WRITE);
-    UtAssert_True(status >= 0, "fd after creat max name length file = %d",(int)status);
-
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
+    status = OS_OpenCreate(&fd, maxfilename, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
+    UtAssert_True(status == OS_SUCCESS, "fd after creat max name length file = %d",(int)status);
 
     /* close the second file */
     status = OS_close(fd);
@@ -177,7 +171,7 @@ void TestCreatRemove(void)
     UtAssert_True(status == OS_SUCCESS, "status after remove max name length file = %d",(int)status);
 
     /* try creating with file name too big, should fail */
-    status = OS_creat(longfilename,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd, longfilename, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status < OS_SUCCESS, "status after create file name too long = %d",(int)status);
 
     /* try removing with file name too big. Should Fail */
@@ -207,25 +201,19 @@ void TestOpenClose(void)
     filename[sizeof(filename) - 1] = 0;
 
     /* create a file of reasonable length (but over 8 chars) */
-    status = OS_creat(filename,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd, filename, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
-
     /*
     ** try to close the file
     */
     status = OS_close(fd);
     UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
 
     /*  reopen the file */
-    status = OS_open(filename,OS_READ_WRITE,0644);
+    status = OS_OpenCreate(&fd, filename, OS_FILE_FLAG_NONE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after reopen = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
-
     /*
     ** try to close the file again
     */
@@ -243,7 +231,7 @@ void TestOpenClose(void)
     UtAssert_True(status != OS_SUCCESS, "status after close = %d",(int)status);
 
     /*  open a file that was never in the system */
-    status = OS_open("/drive0/FileNotHere",OS_READ_ONLY,0644);
+    status = OS_OpenCreate(&fd, "/drive0/FileNotHere", OS_FILE_FLAG_NONE, OS_READ_ONLY);
     UtAssert_True(status < OS_SUCCESS, "status after open = %d",(int)status);
 
     /* try removing the file from the drive  to end the function */
@@ -279,11 +267,9 @@ void TestReadWriteLseek(void)
     /* create a file of reasonable length (but over 8 chars) */
 
     /* Open In R/W mode */
-    status = OS_creat(filename,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd, filename, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
     size = strlen(buffer);
 
     /* test write portion of R/W mode */
@@ -310,12 +296,9 @@ void TestReadWriteLseek(void)
     UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
 
     /*  open a file again, but only in READ mode */
-    status = OS_open(filename,OS_READ_ONLY,0644);
+    status = OS_OpenCreate(&fd, filename, OS_FILE_FLAG_NONE, OS_READ_ONLY);
     UtAssert_True(status >= OS_SUCCESS, "status after reopen = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
-
     /* test write in READ ONLY mode */
     status = OS_write(fd, (void*)buffer, size);
     UtAssert_True(status < OS_SUCCESS, "status after write = %d",(int)status);
@@ -349,12 +332,9 @@ void TestReadWriteLseek(void)
     UtAssert_True(status == OS_SUCCESS, "status after close = %d",(int)status);
 
     /*  open a file again, but only in WRITE mode */
-    status = OS_open(filename,OS_WRITE_ONLY,0644);
+    status = OS_OpenCreate(&fd, filename, OS_FILE_FLAG_NONE, OS_WRITE_ONLY);
     UtAssert_True(status >= OS_SUCCESS, "status after reopen = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd = OS_ObjectIdFromInteger(status);
-
     /* test write in WRITE ONLY mode */
     status = OS_write(fd, (void*)buffer, size);
     UtAssert_True(status == size, "status after write = %d size = %d",(int)status, (int)size);
@@ -423,18 +403,12 @@ void TestMkRmDirFreeBytes(void)
 
     /* now create two files in the two directories (1 file per directory) */
 
-    status = OS_creat(filename1,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd1, filename1, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 1 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd1 = OS_ObjectIdFromInteger(status);
-
-    status = OS_creat(filename2,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd2, filename2, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 2 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd2 = OS_ObjectIdFromInteger(status);
-
     /* write the propper buffers into each of the files */
     size = strlen(buffer1);
     status = OS_write(fd1, buffer1, size);
@@ -537,18 +511,12 @@ void TestOpenReadCloseDir(void)
 
     /* now create two files in the two directories (1 file per directory) */
 
-    status = OS_creat(filename1,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd1, filename1, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 1 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd1 = OS_ObjectIdFromInteger(status);
-
-    status = OS_creat(filename2,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd2, filename2, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 2 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd2 = OS_ObjectIdFromInteger(status);
-
     /* write the proper buffers into each of the files */
     size = strlen(buffer1);
     status = OS_write(fd1, buffer1, size);
@@ -756,12 +724,9 @@ void TestRename(void)
 
     /* now create a file in the directory */
 
-    status = OS_creat(filename1,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd1, filename1, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 1 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd1 = OS_ObjectIdFromInteger(status);
-
     /* write the propper buffes into  the file */
 
     size = strlen(buffer1);
@@ -787,12 +752,9 @@ void TestRename(void)
 
     /* try to read the new file out */
 
-    status = OS_open(newfilename1,OS_READ_ONLY,0644);
+    status = OS_OpenCreate(&fd1, newfilename1, OS_FILE_FLAG_NONE, OS_READ_ONLY);
     UtAssert_True(status >= OS_SUCCESS, "status after open 1 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd1 = OS_ObjectIdFromInteger(status);
-
     size  = strlen(copybuffer1);
     status = OS_read(fd1,buffer1,size);
     UtAssert_True(status == size, "status after read 1 = %d size = %d",(int)status, (int)size);
@@ -838,12 +800,9 @@ void TestStat(void)
     UtAssert_True(status == OS_SUCCESS, "status after mkdir 1 = %d",(int)status);
 
     /* now create a file  */
-    status = OS_creat(filename1,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd1, filename1, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 1 = %d",(int)status);
 
-    /* conversion to osal_id_t */
-    fd1 = OS_ObjectIdFromInteger(status);
-
     /* Write some data into the file */
 
     size = strlen(buffer1);
@@ -887,21 +846,22 @@ void TestOpenFileAPI(void)
     char filename2 [OS_MAX_PATH_LEN];
     char filename3 [OS_MAX_PATH_LEN];
     int status;
+    osal_id_t fd;
 
     strcpy(filename1,"/drive0/Filename1");
     strcpy(filename2,"/drive0/Filename2");
     strcpy(filename3,"/drive0/Filename3");
 
     /* Create/open a file */
-    status = OS_creat(filename1,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd, filename1, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 1 = %d",(int)status);
 
     /* Create/open a file */
-    status = OS_creat(filename2,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd, filename2, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 2 = %d",(int)status);
 
     /* Create/open a file */
-    status = OS_creat(filename3,OS_READ_WRITE);
+    status = OS_OpenCreate(&fd, filename3, OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
     UtAssert_True(status >= OS_SUCCESS, "status after creat 3 = %d",(int)status);
 
     /* 

diff --git a/src/unit-test-coverage/shared/src/coveragetest-file.c b/src/unit-test-coverage/shared/src/coveragetest-file.c
@@ -48,41 +48,45 @@ void Test_OS_FileAPI_Init(void)
     UtAssert_True(actual == expected, "OS_FileAPI_Init() (%ld) == OS_SUCCESS", (long)actual);
 }
 
-void Test_OS_creat(void)
+void Test_OS_OpenCreate(void)
 {
     /*
      * Test Case For:
-     * int32 OS_creat  (const char *path, int32  access)
+     * int32 OS_OpenCreate(osal_id_t *filedes, const char *path, int32 flags, int32 access)
      */
-    int32 actual = OS_creat("/cf/file", OS_READ_WRITE);
-    UtAssert_True(actual >= 0, "OS_creat() (%ld) >= 0", (long)actual);
+    int32 expected;
+    int32 actual;
+    osal_id_t filedes;
 
-    actual = OS_creat("/cf/file", OS_READ_ONLY);
-    UtAssert_True(actual == OS_ERROR, "OS_creat() (%ld) == OS_ERROR", (long)actual);
+    /* Test in OS_creat mode */
+    expected = OS_SUCCESS;
+    actual = OS_OpenCreate(&filedes, "/cf/file", OS_FILE_FLAG_CREATE | OS_FILE_FLAG_TRUNCATE, OS_READ_WRITE);
+    UtAssert_True(actual == expected, "OS_OpenCreate() (%ld) == OS_SUCCESS (create mode)", (long)actual);
 
-    UT_SetForceFail(UT_KEY(OS_TranslatePath), OS_ERROR);
-    actual = OS_creat(NULL, OS_WRITE_ONLY);
-    UtAssert_True(actual == OS_ERROR, "OS_creat() (%ld) == OS_ERROR", (long)actual);
-    UT_ClearForceFail(UT_KEY(OS_TranslatePath));
+    /* Test in OS_open mode */
+    actual = OS_OpenCreate(&filedes, "/cf/file", OS_FILE_FLAG_NONE, OS_READ_WRITE);
+    UtAssert_True(actual == expected, "OS_OpenCreate() (%ld) == OS_SUCCESS (open mode)", (long)actual);
 
-}
+    /* Test with bad descriptor buffer */
+    expected = OS_INVALID_POINTER;
+    actual = OS_OpenCreate(NULL, "/cf/file", OS_FILE_FLAG_NONE, OS_READ_WRITE);
+    UtAssert_True(actual == expected, "OS_OpenCreate() (%ld) == OS_INVALID_POINTER (bad buffer)", (long)actual);
 
-void Test_OS_open(void)
-{
-    /*
-     * Test Case For:
-     * int32 OS_open   (const char *path,  int32 access,  uint32  mode)
-     */
-    int32 actual = OS_open("/cf/file", OS_READ_WRITE, 0);
+    /* Test with bad access flags */
+    expected = OS_ERROR;
+    actual = OS_OpenCreate(&filedes, "/cf/file", OS_FILE_FLAG_NONE, 9999);
+    UtAssert_True(actual == expected, "OS_OpenCreate() (%ld) == OS_ERROR (bad flags)", (long)actual);
 
-    UtAssert_True(actual > 0, "OS_open() (%ld) > 0", (long)actual);
 
+    /* Test failure to convert path */
+    UT_SetForceFail(UT_KEY(OS_TranslatePath), OS_ERROR);
+    expected = OS_ERROR;
+    actual = OS_OpenCreate(&filedes, "/cf/file", OS_FILE_FLAG_NONE, OS_READ_WRITE);
+    UtAssert_True(actual == OS_ERROR, "OS_OpenCreate() (%ld) == OS_ERROR (bad path)", (long)actual);
+    UT_ClearForceFail(UT_KEY(OS_TranslatePath));
 
-    actual = OS_open("/cf/file", -1, 0);
-    UtAssert_True(actual < 0, "OS_open() (%ld) < 0", (long)actual);
 }
 
-
 void Test_OS_close(void)
 {
     /*
@@ -344,10 +348,6 @@ void Test_OS_FDGetInfo(void)
     actual = OS_FDGetInfo(UT_OBJID_1, &file_prop);
 
     UtAssert_True(actual == expected, "OS_FDGetInfo() (%ld) == OS_SUCCESS", (long)actual);
-#ifdef jphfix
-    UtAssert_True(file_prop.User == 111, "file_prop.User (%lu) == 111",
-            (unsigned long)file_prop.User);
-#endif
     UtAssert_True(strcmp(file_prop.Path, "ABC") == 0, "file_prop.Path (%s) == ABC",
             file_prop.Path);
 
@@ -457,8 +457,7 @@ void Osapi_Test_Teardown(void)
 void UtTest_Setup(void)
 {
     ADD_TEST(OS_FileAPI_Init);
-    ADD_TEST(OS_creat);
-    ADD_TEST(OS_open);
+    ADD_TEST(OS_OpenCreate);
     ADD_TEST(OS_close);
     ADD_TEST(OS_TimedRead);
     ADD_TEST(OS_TimedWrite);