Skip to content

Commit

Permalink
Fix #556, Deprecate OS_open and OS_creat
Browse files Browse the repository at this point in the history 
These functions are replaced by OS_OpenCreate, which
implements both functions via flags, and follows the
correct OSAL API patterns.
  • Loading branch information
@jphickey
jphickey committed Oct 2, 2020
1 parent 27c2469 commit 1776749
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 179 deletions.
10 changes: 10 additions & 0 deletions src/os/inc/osapi-os-filesys.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ typedef enum
* @{
*/

#ifndef OSAL_OMIT_DEPRECATED

/*-------------------------------------------------------------------------------------*/
/**
* @brief Creates a file specified by path
Expand All @@ -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);

Expand All @@ -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
Expand Down
23 changes: 22 additions & 1 deletion src/os/shared/src/osapi-file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -205,7 +226,7 @@ int32 OS_open (const char *path, int32 access, uint32 mode)
return return_code;
} /* end OS_open */


#endif

/*----------------------------------------------------------------
*
Expand Down
84 changes: 22 additions & 62 deletions src/tests/file-api-test/file-api-test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 */
Expand Down Expand Up @@ -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
*/
Expand All @@ -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 */
Expand Down Expand Up @@ -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 */
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

/*
Expand Down
55 changes: 27 additions & 28 deletions src/unit-test-coverage/shared/src/coveragetest-file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
/*
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 1776749

Please sign in to comment.