-
Notifications
You must be signed in to change notification settings - Fork 267
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
Static analyzer-found cleanup opportunities #155
base: master
Are you sure you want to change the base?
Conversation
Thanks! I've never used the clang static analyzer. I'll look into proposed modification and merge them. |
@@ -125,12 +125,10 @@ ansi_to_markup( char *sequence, size_t length, markup_t *markup ) | |||
else if( (set_fg == 0) && (code == 5) ) | |||
{ | |||
set_fg = 1; | |||
code = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line prevents from ever entering the test just after. Do you know how the analyzer decides this can be safely removed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Observe line 177, code gets assigned 0 outside the if-else statement anyway, directly after line 128 is executed. So line 128 is redundant it seems.
demos/distance-field-3.c
Outdated
float glyph_height = glyph->height * width/(float)glyph->width; | ||
float glyph_width = glyph->width * height/(float)glyph->height; | ||
int x = -glyph_width/2 + width/2.; | ||
int y = -glyph_height/2 + height/2.; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if not used, this is relevant information. Maybe commenting those line would be better, no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll push a change with them added back, but commented out!
Thanks for the feedback. Adjustments made. This library is very useful by the way! |
* Used utf8_strlen on every code-point in the string even for strings with length provided
@ryandrake08 Thanks for the fix. Just tell me you're ready for a merge. |
Feel free to merge whenever. This PR is turning into a grab bag of warnings fixes and cleanups. The only change that may actually change the functionality of the library is 3df29ce. Please have a look at what this one is doing. You may prefer I leave this out. I found I needed it in my application to improve the look of my rendered text. Without this padding I was seeing undesired graphic artifacts around the edges of each glyph. |
A couple of issues found using clang's static analyzer. Most are pretty trivial (removal of unnecessary / unused code). Two moderately concerning ones:
Feel free to merge if you think it's helpful.