Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Fix various GCC warnings #79

Closed
wants to merge 2 commits into from
Closed

Conversation

JosiahWI
Copy link
Contributor

Minor code quality improvements. The main feature is that some unused functions were removed.

@JosiahWI JosiahWI mentioned this pull request Nov 10, 2021
@JosiahWI
Copy link
Contributor Author

JosiahWI commented Nov 10, 2021

There are a number of warnings in the Mac OS CI build yet. Some of these aren't as clean to fix nicely - I'm looking at the hidden virtual method warnings. The same warnings affect OpenBSD (which we have users on) and presumably other BSD variants. I am happy to fix these, but I do not want to burden the devs or go against the project goals. Please advise. Thank you!

@JosiahWI
Copy link
Contributor Author

Clang issues those warnings also.

@JosiahWI
Copy link
Contributor Author

I am not planning to add any more new fixes. This is everything that I can test on my Debian dev workstation.

@sfan5
Copy link
Member

sfan5 commented Nov 19, 2021

Can you split/squash this into two commits? One that only removes unused code and one with the rest.

@JosiahWI
Copy link
Contributor Author

Is there a convenient way to do this in git or do I have to manually recommit the changes? I will force push once the branch tip is re-organized.

@sfan5
Copy link
Member

sfan5 commented Nov 20, 2021

If you have commits separated so that each would fit into either of the final commits you can use git rebase -i, otherwise just recommit (I can recommend git commit -p for picking changes).

renderLine16_Blend(), renderLine16_Decal(), renderLine32_Blend() renderLine32_Decal() functions are now unused.

remove clipLine()

Another unused function.

remove frand()

The frand() computation implicitly cast the rMax integer to a float and in doing so unintentionally increased it. frand() is not used by us, so deleting it is a good solution. The entire randomizer could possibly be removed, but that is not within the scope of this commit.

remove unused private fields

I have removed unused private fields and their constructors, and one unused constant.

remove unused software driver functions

drawRectangle() and drawLine() have been removed, because IrrlichtMt's software drivers have been removed.
Overloading a method in a subclass hides the definitions of the superclass. I don't think this is causing an issue because the subclass explicitly overrides both of the superclass methods, but now there is no question. There are some vptr errors ubsan complains about which this may be related to, but I do not know.

handle missing enumeration values in switch

ESNRP_LIGHT and ESNRP_SHADOW were not handled in the scene node registration. They are now explicitly ignored.

remove extraenous semicolons

always have defaults in color converter switch

The defaults were enabled only in release. Rationale for changing this is that the behavior is the same whether or not default is explicit. So disabling defaults for debugging is not really helpful, except that it displays a warning.

fix root cause of stringop warning

After researching the cause of the warning thoroughly, I realized that GCC is noticing the array[length] = 0 anti-pattern, where it expects array[length - 1] = 0. Sure enough, setting an intermediate variable to hold the allocation length (including the null terminator) lets us use the standard idom, which GCC has no qualms with.
@sfan5
Copy link
Member

sfan5 commented Nov 24, 2021

d4119ba 6d133e1

@sfan5 sfan5 closed this Nov 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants