vxworks osfileapi.c functions not using semaphore

[skliper]

Multiple functions in the VxWorks osfileapi.c are not taking the semaphore when accessing the shared table.

OS_close, OS_read, OS_write, OS_lseek, OS_remove, OS_rename, OS_cp, OS_mv, OS_ShellOutputToFile, OS_FDGetInfo.

Identified with #45 white-box coverage testing.

[skliper]

Imported from trac issue 83. Created by abrown4 on 2015-07-17T14:18:52, last modified: 2015-12-01T14:19:51

[skliper]

Trac comment by abrown4 on 2015-07-28 15:34:49:

Investigation:
These functions do take a mutex for the shared OS_FDTable: OS_creat, OS_open, OS_close, OS_FileOpenCheck, OS_CloseFileByName, OS_CloseAllFiles.

These functions do not use a mutex for the shared OS_FDTable, but they DO alter that table's contents: OS_remove, OS_rename, OS_cp, OS_mv.

These functions do not use a mutex for the shared OS_FDTable, but merely access/read the table elements: OS_read, OS_write, OS_lseek, OS_ShellOutputToFile, OS_FDGetInfo. Given the decision on #103, these functions that only use the table entries will not be altered.

These functions do not touch the shared OS_FDTable at all: **OS_chmod, OS_stat, OS_mkdir, OS_opendir, OS_closedir, OS_readdir, OS_rewinddir, OS_rmdir, OS_check_name_length. ** These will not be altered (obviously).

[skliper]

Trac comment by abrown4 on 2015-07-28 15:48:29:

Noted: OS_open and OS_creat give up the table mutex around the open(). OS_close takes the mutex after the close(). A good (safe) approach.

But OS_CloseFileByName and OS_CloseAllFiles peform the close() inside the mutex lock, unlike OS_close. This is a difference in design & implementation - will change to match the (safer) OS_close.

OS_remove simply needs to check if the file is already not in the table, so no changes required. Same with OS_cp & OS_mv.

OS_rename alters the shared table entry's path without locking and after the rename() is called. There is no check for if the file is in use, like in OS_cp, OS_mv, or OS_remove.

[skliper]

Trac comment by abrown4 on 2015-07-28 16:48:56:

commit: [changeset:76b7d35] Trac #106, Fixed some osfileapi.c global table accesses.
OS_CloseFileByName() and OS_CloseAllFiles() peform the syscall close()
inside the mutex lock. Changed to be like OS_close(), where close() is
called outside the lock.

[skliper]

Trac comment by abrown4 on 2015-07-28 17:00:14:

Opened #116 for the OS_rename behavior.

No other changes expected on this issue (before review & discussion).

[skliper]

Trac comment by abrown4 on 2015-08-20 18:46:24:

From the above commits, as of #45 [changeset:6a9db70], the white-box unit test still shows that many functions don't safely access the OS_FDTable (31 test failures from osfile_wb test).

[skliper]

Trac comment by abrown4 on 2015-11-25 16:10:57:

Traceability: The above [changeset:76b7d35] is reflected in #138 [changeset:ab2d6d7]. Recommend closing this issue in favor of #138 for these improvements to be merged.

However, osfilesys.c is one of the most thread-unsafe parts of OSAL so recommend the CCB discuss paths forward with this issue providing a reference.