Skip to content

Conversation

bcoconni
Copy link
Contributor

On Fedora 32, gcc 10.2.1 reports an error:

 Compiling file src/SDL/MyOpenGLView.m ...
src/SDL/MyOpenGLView.m: In function ‘-[MyOpenGLView init]’:
src/SDL/MyOpenGLView.m:327:2: error: format not a string literal and no format arguments [-Werror=format-security]
  327 |  OOLog(@"display.initGL", @"Achieved color / depth buffer sizes (bits):");
      |  ^
cc1obj: some warnings being treated as errors

This PR fixes the issue on my system.

@AnotherCommander
Copy link
Member

AnotherCommander commented Oct 26, 2020

Begin rant: Why on Earth is this treated as an error on Fedora? It is perfectly legal Obj-C syntax and should at the worst case be flagged as a warning. The proposed fix works of course (and it is not the first tine we've had to apply something like this either) but the code is more confusing and less readable after it has been applied. And, just to be clear, I am not trying to be harsh to anyone, just trying to understand why some versions of gcc seem to be so extremely pedantic, to the point of refusing legal syntax.

Another fix that will work perfectly fine and without any security risk would be to set the -Wno-error=format-security flag somewhere in GNUmakefile.

End rant. Phew. :-)

Since we have already applied similar things in the past, this PR will most likely make it to merge. Just to be sure that there will be no more of those though, was that the only occurrence? Does the build succeed after you have corrected it?

@bcoconni
Copy link
Contributor Author

Another fix that will work perfectly fine and without any security risk would be to set the -Wno-error=format-security flag somewhere in GNUmakefile.

Yes, but I don't feel comfortable disabling security related flags even when they seem pedantic. Furthermore, a similar issue has already been discussed in the PR #198 but the subject has been dropped at the time. However the changes have eventually found their way to the code (see commit 0b12ed3 for example). So, long story short, I have submitted this PR.

Since we have already applied similar things in the past, this PR will most likely make it to merge. Just to be sure that there will be no more of those though, was that the only occurrence? Does the build succeed after you have corrected it?

Yes, the build succeeds once the attached patch is applied and oolite runs smoothly 😄

@AnotherCommander AnotherCommander merged commit d3f5d40 into OoliteProject:master Oct 27, 2020
@bcoconni bcoconni deleted the fix-gcc-error-format-security branch October 27, 2020 20:44
KonstantinosSykas pushed a commit that referenced this pull request Nov 14, 2020
This fixes something that is neither invalid nor a security vulnerability, Still, distros other than Fedora may decide to report it in the future as an error too. So we are getting ahead of them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants