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

Direct use of POSIX permission setting in UT assert #1300

Closed
jphickey opened this issue Oct 4, 2022 · 0 comments · Fixed by #1301
Closed

Direct use of POSIX permission setting in UT assert #1300

jphickey opened this issue Oct 4, 2022 · 0 comments · Fixed by #1301

Comments

@jphickey
Copy link
Contributor

jphickey commented Oct 4, 2022

Describe the bug
The UT assert framework is supposed to be portable, and this should adhere to strict C99.

However, in the goal of squelching a bogus CodeQL warning - some POSIX-specific logic got added in #827. This seemed to work (likely because it was never run on a non-POSIX OS) until it was changed to use a different POSIX API (fstat/fchmod) to address another bogus CodeQL warning.

Now, builds are failing because the APIs used are marked as POSIX-specific but we are not compiling this code with _POSIX_C_SOURCE (nor should we, because its supposed to be pure C99).

This is currently breaking the mainline build.

To Reproduce
Build UT assert - get the error:

/home/runner/work/CF/CF/osal/ut_assert/src/uttools.c: In function ‘UtMem2BinFile’:
/home/runner/work/CF/CF/osal/ut_assert/src/uttools.c:63:14: error: implicit declaration of function ‘fileno’; did you mean ‘fopen’? [-Werror=implicit-function-declaration]
         fd = fileno(fp);
              ^~~~~~
              fopen
/home/runner/work/CF/CF/osal/ut_assert/src/uttools.c:66:13: error: implicit declaration of function ‘fchmod’; did you mean ‘chmod’? [-Werror=implicit-function-declaration]
             fchmod(fd, dststat.st_mode & ~(S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH | S_IXOTH));
             ^~~~~~
             chmod

This is directly from the validation workflow on a recent CF change.

System observed on:
Ubuntu (various)

Additional context
The CodeQL warnings about file permissions "not being set" are not really valid. UNIX still allows file permissions to be set by the user, in the form of the umask setting. So the permissions on a file created using pure C99 stdio calls (fopen) are not "unset" - they are just not set directly by the application, they are controlled externally.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the bug label Oct 4, 2022
jphickey added a commit to jphickey/osal that referenced this issue Oct 4, 2022
File permissions in general are a POSIX concept, but this tool should be
pure C99.  It should not rely on any POSIX headers or POSIX-specific
API calls.
jphickey added a commit to jphickey/osal that referenced this issue Oct 4, 2022
File permissions in general are a POSIX concept, but this tool should be
pure C99.  It should not rely on any POSIX headers or POSIX-specific
API calls.
dmknutsen added a commit that referenced this issue Oct 4, 2022
Fix #1300, do not set file permissions on UT assert outputs
@dmknutsen dmknutsen added this to the Draco milestone Jan 18, 2023
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