vxworks osfilesys.c functions not thread-safe

[skliper]

During Trac #45 unit testing it was found that many functions in the vxworks osfilesys.c are not using the semaphore to access the shared table.

Demonstrated on the trac-22 branch, [changeset:0f36fb6]

[skliper]

Imported from trac issue 85. Created by abrown4 on 2015-07-24T20:19:03, last modified: 2019-07-26T15:13:27

[skliper]

Trac comment by abrown4 on 2015-08-20 14:40:22:

Noted in the 8/19/2015 vx works code inspection review (trac #45) that the tables used by osfilesys.c are extern and from the PSP. But not locking/thread-safe protection exists.

[skliper]

Trac comment by abrown4 on 2015-08-20 18:48:21:

From the above commits, as of #45 [changeset:6a9db70], the white-box unit test shows that functions don't safely access the tables (36 out of 38 test failures from osfilesys_wb test apply to this issue).

[skliper]

Trac comment by jhageman on 2019-02-28 15:12:49:

Please confirm condition still exists, and if so provide estimated work complete date or detail what external inputs are needed to be able to provide a work complete date.

[skliper]

Trac comment by jphickey on 2019-03-08 12:42:32:

I am going to take this one.

I actually noticed the same thing when doing the implementation of the vxworks-ng layer as part of #231. None of the file system volume table manipulations are adequately protected.

Even in the shared/common layer that was implemented in #28, the filesystem calls continue to use the OS_VolumeTable for backward compatibility, but the backward compatibility unfortunately includes this bug too.

However, as part of doing the vxworks-ng layer, I fixed it for the shared/common layer. So my fix applies to posix-ng, rtems-ng, and vxworks-ng, but not the classic vxworks6, posix, or rtems implementations.

[skliper]

Trac comment by jphickey on 2019-03-15 13:17:24:

The first pass at this is ready for CCB review.

This ended up being a fairly substantial change to the file system layer due to a bit of "domino effect". However, I do think that all changes are worthwhile despite the high (-ish) number of lines affected.

All changes are constrained to the "ng" OSAL (the shared layer along with posix-ng/rtems-ng implementations). It is also fully backward compatible as no external API calls were changed.

Also worth noting: With this change, the OS_VolumeTable is no longer used for actively storing the state information during runtime. The table can now be considered const as it only provides hints to the implementation during filesystem instantiation (i.e. the type of filesystem to use).

The CCB has discussed possibly phasing out OS_VolumeTable and this takes an important step in that direction. For now, it still uses it for backward compatibility with existing BSPs, but this could be made optional now.

Proposed for review: commit [changeset:b2692bb] branch trac-85-rework-filesys-layer

[skliper]

Trac comment by jphickey on 2019-03-26 09:22:20:

Adding this to the group of bugs that are fixed by using the "NG" architecture

[skliper]

Trac comment by jphickey on 2019-06-10 13:08:33:

Update: For review purposes this change has been split into two parts.

There is a new ticket, #242, for the general shared layer changes. This is a prerequisite to this change, and should be reviewed/merged first.

After this, the updates that specifically address the issues in the filesystem API layer can be reviewed in commit [changeset:679763b]

[skliper]

Trac comment by jphickey on 2019-06-10 16:54:41:

Updated commit (again) incorporating review comments from 2019-06-10:

Commit [changeset:9fb0802]

[skliper]

Trac comment by jphickey on 2019-06-12 12:37:47:

Update after fixing merge/rebase issue: [changeset:e32facb]

[skliper]

Trac comment by jhageman on 2019-06-13 11:25:53:

CCB 6/12/19 - CCB code reviewed and approved

[skliper]

Trac comment by jphickey on 2019-06-25 11:52:28:

Update: cppcheck static analysis (from bamboo integration tests) noted an unused variable:

osal/src/os/shared/osapi-filesys.c:536: style: Variable 'local' is assigned a value that is never used.

Commit [changeset:3a9d79cc] corrects this. Re-merged into ic-ccb-20190612

[skliper]

Trac comment by jhageman on 2019-07-10 13:23:34:

CCB 07/10/2019 - Approved for merge to dev

[skliper]

Trac comment by jphickey on 2019-07-10 13:46:02:

Now in the "development" branch

[skliper]

Trac comment by jhageman on 2019-07-26 15:13:27:

Milestone renamed