You are not logged in.
I'm trying to help to improve oolite sourcecode and felt the best way to start is to eliminate the need to build with -Wno-format-security.
The first error when building oolite-git from AUR without that flag is
src/SDL/main.m: In function ‘main’:
src/SDL/main.m:171:45: error: format not a string literal and no format arguments [-Werror=format-security]
171 | OO_SHOW_MSG(s, processName, MB_OK);
| ^
src/SDL/main.m:119:73: note: in definition of macro ‘OO_SHOW_MSG’
119 | #define OO_SHOW_MSG(ooMsg, ooTitle, ooFlags) fprintf(stdout, ooMsg)
| ^~~~~
cc1obj: some warnings being treated as errors
The file in q is https://github.com/OoliteProject/oolite … SDL/main.m .
I do understand that formatting should be added but have no idea which format specifier would work.
Please help.
Last edited by Lone_Wolf (2025-08-31 10:48:45)
Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
clean chroot building not flexible enough ?
Try clean chroot manager by graysky
Offline
fprintf(stdout, "%s", ooMsg)
Not sure whether the string literal requirement is entirely correct on gcc's part, though.
Online
This specific warning is thrown because the compiler can not check whether the resulting format string is valid (in accordance with the supplied arguments).
In this case the process name could contain a format specifier e.g. %s and cause fprintf to format an empty parameter list which pops a random value off the stack.
Try this example:
printf("%s\n");
As snprintf writes the null byte in the buffer it should not overflow but could truncate the output if the process name is to long.
Also the other parameters are silently discarded, which is fine but could be added for consistency.
It is not a requirement hence why it throws a warning.
Offline
This specific warning is thrown because the compiler can not check whether the resulting format string is valid (in accordance with the supplied arguments).
In this case the process name could contain a format specifier e.g. %s and cause fprintf to format an empty parameter list which pops a random value off the stack.
Also the other parameters are silently discarded, which is fine but could be added for consistency.
src/SDL/main.m:171:45: error: format not a string literal and no format arguments [-Werror=format-security]
171 | OO_SHOW_MSG(s, processName, MB_OK);
I think you have s and processName mixed up. s is the output from snprintf https://github.com/OoliteProject/oolite … ain.m#L150 so is already length checked and null terminated. From that it could be argued to just ignore the warning for this specific instance.
Offline
It is not a requirement hence why it throws a warning.
Archlinux default flags include -Werror=format-security so it's no longer considered a warning.
OO_SHOW_MSG is a macro and defined on line 117 of the file
#define OO_SHOW_MSG(ooMsg, ooTitle, ooFlags) fprintf(stdout, ooMsg)
I guess it is the fprintf(stdout, ooMsg) part that needs to be changed ?
EDIT : thx for the confirmation, seth.
Last edited by Lone_Wolf (2025-08-26 22:03:39)
Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
clean chroot building not flexible enough ?
Try clean chroot manager by graysky
Offline
fprintf(stdout, "%s", ooMsg)
Not sure whether the string literal requirement is entirely correct on gcc's part, though.
Online
Not sure whether the string literal requirement is entirely correct on gcc's part, though.
ooMsg is s which is char s[2048]; so it should be a string literal?
Offline
"man 3{p} printf" demands a const char*, so any demand for a string literal here is dubious tbw. (but apparently gcc seeks to validate the patter, variadic parameters) but while every string literal is a char[] not every char[] is a string literal. Technically the latter needs to be const and in practice, it's only "foo"
Notably char s[2048] can go out of context any time.
See eg. https://stackoverflow.com/questions/127 … char-array for speculation and discussions and explanations, but gcc obviously reads "string literal" as "letters between quotes" here.
Online
Replace `fprintf` with `fwrite` as s is already a formatted string and avoid the issue?
fwrite(ooMsg, sizeof(ooMsg), 1, stdout);
Offline
Replace `fprintf` with `fwrite` as s is already a formatted string and avoid the issue?
fwrite(ooMsg, sizeof(ooMsg), 1, stdout);
Depending on macro substitution context (whether ooMsg array decay to a pointer to first element or not) this will write either entire buffer with nul terminator and subsequent garbage, or sizeof(pointer) bytes.
This should work:
fwrite(ooMsg, strlen(ooMsg), 1, stdout);
However, printf() with constant format string seems better:
#define OO_SHOW_MSG(ooMsg, ooTitle, ooFlags) fprintf(stdout, "%s", ooMsg)
Last edited by dimich (2025-08-27 06:06:12)
Offline
I think you have s and processName mixed up. s is the output from snprintf https://github.com/OoliteProject/oolite … ain.m#L150 so is already length checked and null terminated. From that it could be argued to just ignore the warning for this specific instance.
I don't think that I have them mixed up (correct me if I'm wrong).
What I mainly was getting at is that the process name could contain a format specifier. Usually the process name is derived from the program file name, but can be adjusted at runtime [1].
The problem that could arise if the binary is named something like 'program%s' and that would be passed to fprintf which would then miss an argument and pop a random value of the stack.
This is just a minor bug and not really important but I wanted to point it out.
I agree with the constant '%s' specifier which sidesteps this problem.
[1] https://developer.gnustep.org/APIRefs/B … rocessName
Edit: Highlight solution
Last edited by lmn (2025-08-27 12:18:16)
Offline
Using fprintf(stdout, "%s", ooMsg) fixed that warning, next one looks trickier.
src/SDL/MyOpenGLView.m
src/SDL/MyOpenGLView.m: In function ‘-[MyOpenGLView snapShot:]’:
src/SDL/MyOpenGLView.m:1757:25: error: format not a string literal and no format arguments [-Werror=format-security]
1757 | OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to "SNAPSHOTHDR_EXTENSION_DEFAULT);
| ^
OOLog is used multiple times in this file, defined at lines 94 to 106
#if OOLITE_LINUX
SDL_SysWMinfo dpyInfo;
SDL_VERSION(&dpyInfo.version);
if(SDL_GetWMInfo(&dpyInfo))
{
nativeDisplayWidth = DisplayWidth(dpyInfo.info.x11.display, 0);
nativeDisplayHeight = DisplayHeight(dpyInfo.info.x11.display, 0);
OOLog(@"display.mode.list.native", @"X11 native resolution detected: %d x %d", nativeDisplayWidth, nativeDisplayHeight);
}
else
{
OOLog(@"display.mode.list.native.failed", @"%@", @"SDL_GetWMInfo failed, defaulting to 1024x768 for native size");
}
Last edited by Lone_Wolf (2025-08-27 13:15:45)
Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
clean chroot building not flexible enough ?
Try clean chroot manager by graysky
Offline
https://github.com/OoliteProject/oolite … ew.m#L1757 "Unrecognized HDR file format requested, defaulting to " missing %s at the end.
Edit:
OOLog can already handle format strings https://github.com/OoliteProject/oolite … .h#L88-L91 the previous issue was a special case since OO_SHOW_MSG was only being used in one instance the easier fix was to hardcode the format string to "%s" in OO_SHOW_MSG's definition rather than use `## __VA_ARGS__` and add the format string to the caller.
Last edited by loqs (2025-08-27 13:53:03)
Offline
When adding ,%s at the end a missing expression error came.
Turns out the problem is a missing comma in "SNAPSHOTHDR_EXTENSION_DEFAULT .
No other format security warnings in the sourcecode, going to create an Issue with oolite to get the code fixed.
Thanks for the help.
Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
clean chroot building not flexible enough ?
Try clean chroot manager by graysky
Offline
SNAPSHOTHDR_EXTENSION_DEFAULT is defined as SNAPSHOTHDR_EXTENSION_EXR is defined as @".exr"
The concerned line constructs a string literal using the preprocessor, but the format check apparently doesn't understand that.
OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to ", SNAPSHOTHDR_EXTENSION_DEFAULT);
should still fail but
OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to %s", SNAPSHOTHDR_EXTENSION_DEFAULT);
will work by altering the signature and turning the preprocessor string into the parameter for the "%s" pattern.
Online
Limitation in gcc that it can detect SNAPSHOTHDR_EXTENSION_DEFAULT is a fixed string so does not need a format string in this case:
OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to ", SNAPSHOTHDR_EXTENSION_DEFAULT);
but does not detect the string concatenation in the original?
Offline
Is suspect lone_wolf arrived at
OOLog(@"screenshotHDR", @"Unrecognized HDR file format requested, defaulting to %s", SNAPSHOTHDR_EXTENSION_DEFAULT);
which should™ side-step these issues.
Eg.
printf("foo", "bar")
will (correctly) trigger a format error while
printf("foo %s", "bar")
is fine.
Online
Issue opened and ooliye devs created a PR 526 for it.
One of them added a comment I think many will agree with.
Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
clean chroot building not flexible enough ?
Try clean chroot manager by graysky
Offline
Did %s w/ the NSString actually pass?
Online
Yes, gcc accepted either %s and %@ . The format-security check seems to be not very thorough .
Disliking systemd intensely, but not satisfied with alternatives so focusing on taming systemd.
clean chroot building not flexible enough ?
Try clean chroot manager by graysky
Offline
Well, it thoroughly breaks compilation
Online