Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More test failures related to "chmod" implementation #708

Closed
jphickey opened this issue Dec 28, 2020 · 0 comments · Fixed by #710 or #750
Closed

More test failures related to "chmod" implementation #708

jphickey opened this issue Dec 28, 2020 · 0 comments · Fixed by #710 or #750
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Describe the bug
The current OS_FileChmod_Impl on POSIX has some issues/limitations:

  1. A while back this was changed from using the filename based calls to using a file descriptor, which "protects" against a theoretical issue where the file may get renamed while the operation is in progress. However this introduced a potential for a file descriptor leak -- If the fstat() call fails, the OS_ERROR is returned, but the file descriptor is left open.
  2. Any failure is reported as the generic OS_ERROR ... This should do a better job of translating the errno to a more specific error code. In particular, not all file systems support unix-style file permissions - such as the very common FAT32/DOSFS - and in this case the fchmod() call is likely to return -1 with errno set to something like ENOTSUP or ENOSYS.
  3. Also a file system can be mounted read only, which also prevents chmod() from working.

The main issue of 2+3 above is that the generic OS_ERROR code causes the chmod unit test to fail.

To Reproduce
Run the chmod test on VxWorks using a DOSFS mounted filesystem (e.g. CF:0 on the MCP750 test platform).

Expected behavior
The chmod test cases should be skipped without failing the overall test in cases where the mounted file system does not support/allow permission to be changed.

System observed on:
VxWorks 6.9 on MCP750

Additional context
So this is just a matter of translating the errno values for these conditions into OS_ERR_NOT_IMPLEMENTED instead of OS_ERROR, because the test is already implemented to check for and skip the rest of the test when it gets this return code.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Dec 28, 2020
@jphickey jphickey added the bug label Dec 28, 2020
jphickey added a commit to jphickey/osal that referenced this issue Dec 28, 2020
Better error translations in the OS_FileChmod_Impl() function.
Also corrects a file handle leak.

This makes it return OS_ERR_NOT_IMPLEMENTED when run on a file
system that does not have permissions, which in turn causes the
unit test to be skipped rather than fail.
jphickey added a commit to jphickey/osal that referenced this issue Dec 28, 2020
Better error translations in the OS_FileChmod_Impl() function.
Also corrects a file handle leak.

This makes it return OS_ERR_NOT_IMPLEMENTED when run on a file
system that does not have permissions, which in turn causes the
unit test to be skipped rather than fail.
jphickey added a commit to jphickey/osal that referenced this issue Dec 28, 2020
Better error translations in the OS_FileChmod_Impl() function.
Also corrects a file handle leak.

This makes it return OS_ERR_NOT_IMPLEMENTED when run on a file
system that does not have permissions, which in turn causes the
unit test to be skipped rather than fail.
@astrogeco astrogeco added this to the 6.0.0 milestone Jan 12, 2021
astrogeco added a commit that referenced this issue Jan 12, 2021
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants