Address all Wall warnings #4889
Conversation
|
This pull request introduces 1 alert and fixes 3 when merging e8d8980 into 2f86e36 - view on LGTM.com new alerts:
fixed alerts:
|
| // FIXME Where to set? | ||
| // memcpy(ent->u.splines.stroke.dashes,dashes,sizeof(dashes)); |
There was a problem hiding this comment.
Note: the bot above (@lgtm-com) is registering a new FIXME…I don't know how to use its website and don't care to learn so I checked out 2f86e36 of jtanx/fontforge cf2 branch and then compared it to HEAD (jtanx/fontforge@e8d8980). It's right here. I thought it was probably a bug but it's not, @jtanx really added a FIXME. Probably does not matter, would be good to fix in ideal world, which we don't live in. But, in case anyone else wants to know what bot is complaining about, this is it.
|
@jtanx can you give me a primer or a pointer on |
|
It's a thing from libxml2, probably something to do with unicode support and preventing implicit casts of probably ansi byte strings to what should be utf-8 Edit: ref - http://xmlsoft.org/html/libxml-xmlstring.html#BAD_CAST |
|
Ah, OK. |
|
If/when you resolve the ggdkdraw.c conflict with the since-merged stuff I'll approve this. I didn't look at every line but I assumed the commit comments were accurate and looked at the commits that seemed more likely to introduce a problem. Looked fine to me. |
|
That seems reasonable to me. I was going to try to do every line, but there are a lot of lines! |
|
This pull request introduces 1 alert and fixes 3 when merging 6ad626c into f7a36cc - view on LGTM.com new alerts:
fixed alerts:
|
|
I approved this, but we may want to think about what order to merge it in compared with @linusromer's changes. Not sure we want to create a lot of merge/cleanup work for him. |
Not addressing misleading indentation for now; requires reformatting.
| @@ -2911,7 +2911,7 @@ void SplineSetAddInflections(SplineChar *sc, SplineSet *ss, int anysel) { | |||
| Spline *s, *first; | |||
| first = NULL; | |||
| for ( s = ss->first->next; s!=NULL && s!=first; s = s->to->next ) { | |||
| if ( !anysel || s->from->selected && s->to->selected ) | |||
| if ( !anysel || (s->from->selected && s->to->selected) ) | |||
There was a problem hiding this comment.
@linusromer just confirming that this is the intended behaviour?
There was a problem hiding this comment.
Yes, either nothing is selected or both endpoints are selected. I thought that a || b && c is equivalent to a || (b && c) but I may be misled by other programming languages.
|
This pull request introduces 1 alert and fixes 3 when merging 7b7f763 into 6d63889 - view on LGTM.com new alerts:
fixed alerts:
|
Most of this work was done months ago and a lot of the fixes automated away with some scripting.
This addresses all the warnings at -Wall with my version of gcc. Would also like to tackle Wextra at some point, but this is plenty big as is. Having said that, the changes should all be straightforward, especially if observed commit by commit.
Fixes at least one case of UB.
Type of change