Warning rollup (probably some hidden bugs!) from clang trunk by nabijaczleweli · Pull Request #5492 · fontforge/fontforge

nabijaczleweli · 2024-11-08T18:31:46Z

Type of change

Non-breaking change

These were found in a default build on clang trunk.

I prioritised retaining actual behaviour. There are probably one or two spots (the tautological comparisons, first_cnt most likely) where I actually found and enshrined a bug! Don't apply any of these without validating!

Also:

[66/280] Building C object fontforge/CMakeFiles/fontforge.dir/encoding.c.o
/home/nabijaczleweli/uwu/fontforge/fontforge/encoding.c:2269:28: warning: variable 'refs' used in loop condition not modified in loop body [-Wfor-loop-analysis]
 2269 |             for ( bref=bfc->refs; refs!=NULL; bref = brnext ) {
      |                                   ^~~~
1 warning generated.

this is an infinite loop if refs!=NULL. So refs == NULL always at this point. Someone who understands this code should either remove the loop or fix the condition.

I ignored -Wmaybe-uninitialized. There's so many. Someone who understands the code should build with Clang and see!

[23/280] Building C object fontforge/CMakeFiles/fontforge.dir/crctab.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/crctab.c:49:22: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C23 [-Wdeprecated-non-prototype] 49 | static unsigned long binhex_updcrc(icrc, icp, icnt) | ^ 1 warning generated.

Found with: /home/nabijaczleweli/uwu/fontforge/fontforge/tottfvar.c:588:36: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare] 588 | if ( deltas[i][pts[rj]]>0x7f || deltas[i][pts[rj]]<0x80 ) | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~

Found by [109/280] Building C object fontforge/CMakeFiles/fontforge.dir/featurefile.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3813:7: warning: variable 'cur' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 3813 | if ( contents!=NULL ) { | ^~~~~~~~~~~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3827:5: note: uninitialized use occurs here 3827 | cur->is_mark2base = is_base; | ^~~ /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3813:2: note: remove the 'if' if its condition is always true 3813 | if ( contents!=NULL ) { | ^~~~~~~~~~~~~~~~~~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3803:29: note: initialize the variable 'cur' to silence this warning 3803 | struct markedglyphs *cur; | ^ | = NULL /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3877:7: warning: variable 'cur' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 3877 | if ( contents!=NULL ) { | ^~~~~~~~~~~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3891:5: note: uninitialized use occurs here 3891 | cur->is_mark2lig = true; | ^~~ /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3877:2: note: remove the 'if' if its condition is always true 3877 | if ( contents!=NULL ) { | ^~~~~~~~~~~~~~~~~~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/featurefile.c:3865:29: note: initialize the variable 'cur' to silence this warning 3865 | struct markedglyphs *cur; | ^ | = NULL 2 warnings generated.

[86/280] Building C object fontforge/CMakeFiles/fontforge.dir/parsepfa.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/parsepfa.c:2725:14: warning: comparison of array 'fd->encoding' not equal to a null pointer is always true [-Wtautological-pointer-compare] 2725 | if ( fd->encoding!=NULL ) | ~~~~^~~~~~~~ ~~~~ 1 warning generated. FontDict::encoding is char *encoding[256];

[134/280] Building C object fontforge/CMakeFiles/fontforge.dir/svg.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/svg.c:2805:9: warning: variable 'svg' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 2805 | if (doc != NULL) | ^~~~~~~~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/svg.c:2807:9: note: uninitialized use occurs here 2807 | if (svg != NULL) | ^~~ /home/nabijaczleweli/uwu/fontforge/fontforge/svg.c:2805:5: note: remove the 'if' if its condition is always true 2805 | if (doc != NULL) | ^~~~~~~~~~~~~~~~ 2806 | svg = xmlDocGetRootElement(doc); /home/nabijaczleweli/uwu/fontforge/fontforge/svg.c:2803:19: note: initialize the variable 'svg' to silence this warning 2803 | xmlNodePtr svg; | ^ | = NULL 1 warning generated.

[129/280] Building C object fontforge/CMakeFiles/fontforge.dir/scstyles.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/scstyles.c:5383:9: warning: variable 'i' set but not used [-Wunused-but-set-variable] 5383 | int i; | ^ 1 warning generated.

[136/280] Building C object fontforge/CMakeFiles/fontforge.dir/tottfaat.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/tottfaat.c:299:10: warning: variable 'first_cnt' set but not used [-Wunused-but-set-variable] 299 | int first_cnt = kc->first_cnt; | ^ 1 warning generated.

[30/178] Building C object gdraw/CMakeFiles/gdraw.dir/gfilechooser.c.o /home/nabijaczleweli/uwu/fontforge/gdraw/gfilechooser.c:612:14: warning: explicitly assigning value of variable of type 'int' to itself [-Wself-assign] 612 | dirindex = dirindex; | ~~~~~~~~ ^ ~~~~~~~~ 1 warning generated.

[34/178] Building C object fontforge/CMakeFiles/fontforge.dir/splineoverlap.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/splineoverlap.c:2942:9: warning: variable 'cnt' set but not used [-Wunused-but-set-variable] 2942 | int cnt, ncnt; | ^ 1 warning generated.

[1/132] Building C object gdraw/CMakeFiles/gdraw.dir/hotkeys.c.o /home/nabijaczleweli/uwu/fontforge/gdraw/hotkeys.c:384:14: warning: address of array 'hk->action' will always evaluate to 'true' [-Wpointer-bool-conversion] 384 | if( !hk->action ) | ~~~~~^~~~~~ 1 warning generated. Hotkey::action is char action[HOTKEY_ACTION_MAX_SIZE+1];

[19/124] Building C object fontforge/CMakeFiles/fontforge.dir/splineoverlap.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/splineoverlap.c:2942:9: warning: variable 'ncnt' set but not used [-Wunused-but-set-variable] 2942 | int ncnt; | ^ /home/nabijaczleweli/uwu/fontforge/fontforge/splineoverlap.c:2970:31: warning: variable 'ncnt' is uninitialized when used here [-Wuninitialized] 2970 | if ( ml->m->isneeded ) ++ncnt; | ^~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/splineoverlap.c:2942:13: note: initialize the variable 'ncnt' to silence this warning 2942 | int ncnt; | ^ | = 0 2 warnings generated.

[22/124] Building C object fontforge/CMakeFiles/fontforge.dir/splinesave.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/splinesave.c:2911:9: warning: variable 'unsafecnt' set but not used [-Wunused-but-set-variable] 2911 | int unsafecnt=0, allwithouthints=true; | ^ 2 warnings generated.

[8/77] Building C object fontforgeexe/CMakeFiles/fontforgeexe.dir/prefs.c.o /home/nabijaczleweli/uwu/fontforge/fontforgeexe/prefs.c:2554:9: warning: variable 'line' set but not used [-Wunused-but-set-variable] 2554 | int line = 0,line_max = 3; | ^ 1 warning generated.

[104/280] Building C object fontforge/CMakeFiles/fontforge.dir/parsettf.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/parsettf.c:5189:26: warning: variable 'gcbig' set but not used [-Wunused-but-set-variable] 5189 | int format, len, gc, gcbig, val; | ^ 1 warning generated.

[45/133] Building C object fontforge/CMakeFiles/fontforge.dir/ufo.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/ufo.c:2469:18: warning: variable 'colorp' is uninitialized when used here [-Wuninitialized] 2469 | while (colors[colorp] == ' ' || colors[colorp] == ',') colorp++; | ^~~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/ufo.c:2466:15: note: initialize the variable 'colorp' to silence this warning 2466 | off_t colorp, colorps; | ^ | = 0

[45/133] Building C object fontforge/CMakeFiles/fontforge.dir/ufo.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/ufo.c:3468:10: warning: variable 'member_count' set but not used [-Wunused-but-set-variable] 3468 | int member_count = 0; | ^ 2 warnings generated.

…loop warnings [52/71] Building C object fontforgeexe/CMakeFiles/fontforgeexe.dir/wordlistparser.c.o /home/nabijaczleweli/uwu/fontforge/fontforgeexe/wordlistparser.c:184:47: warning: while loop has empty body [-Wempty-body] 184 | TRACE("have subst. char: %s\n", tmp->name ); break; | ^ /home/nabijaczleweli/uwu/fontforge/fontforgeexe/wordlistparser.c:184:47: note: put the semicolon on a separate line to silence this warning 1 warning generated.

Found by: [154/243] Building C object fontforge/CMakeFiles/fontforge.dir/scripting.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/scripting.c:2344:22: warning: overlapping comparisons always evaluate to false [-Wtautological-overlap-compare] 2344 | if ( c->a.argc<2 && c->a.argc>4 ) { | ~~~~~~~~~~~~^~~~~~~~~~~~~~ 1 warning generated.

Found by clang self-assignment warning

[174/280] Building C object fontforge/CMakeFiles/fontforge.dir/stemdb.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/stemdb.c:2366:32: warning: comparison of address of 'other->to->next' not equal to a null pointer is always true [-Wtautological-pointer-compare] 2366 | topd->colinear && &other->to->next != NULL ) { | ~~~~~~~~~~~^~~~ ~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/stemdb.c:2382:36: warning: comparison of address of 'other->from->prev' not equal to a null pointer is always true [-Wtautological-pointer-compare] 2382 | frompd->colinear && &other->from->prev != NULL ) { | ~~~~~~~~~~~~~^~~~ ~~~~ 2 warnings generated. [175/280] Building C object fontforge/CMakeFiles/fontforge.dir/tottfgpos.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/tottfgpos.c:711:6: warning: variable 'glyph_cnt' set but not used [-Wunused-but-set-variable] 711 | int glyph_cnt = 0; | ^ 1 warning generated.

skef · 2024-11-11T02:54:49Z

 	}
 	/* Now output the corresponding deltas for those points */
 	for ( j=0; j<pcnt; ) {
-	    if ( deltas[i][j]>0x7f || deltas[i][j]<0x80 ) {


These changes obviously don't affect the behavior but they make me nervous. But this is a MultiMaster thing and while that code may be revived at some point for VF support it's effectively dead now.

I tried to make some sense of this but it appears whole-sale in 6243f73 with the initial addition of this file in 2004 and hasn't changed since, so if it behaved correctly when tested then, then clearly it was never intended to run?

It's hard to judge because the multi-master code was almost never used. The implementation is at best incomplete.

@skef

[174/280] Building C object fontforge/CMakeFiles/fontforge.dir/stemdb.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/stemdb.c:2366:32: warning: comparison of address of 'other->to->next' not equal to a null pointer is always true [-Wtautological-pointer-compare] 2366 | topd->colinear && &other->to->next != NULL ) { | ~~~~~~~~~~~^~~~ ~~~~ /home/nabijaczleweli/uwu/fontforge/fontforge/stemdb.c:2382:36: warning: comparison of address of 'other->from->prev' not equal to a null pointer is always true [-Wtautological-pointer-compare] 2382 | frompd->colinear && &other->from->prev != NULL ) { | ~~~~~~~~~~~~~^~~~ ~~~~ 2 warnings generated. @skef commented on this pull request. > @@ -2362,8 +2362,7 @@ return( 0 ); /* fonts or fonts with quadratic splines). */ /* But do that only for colinear spline segments and ensure that there are */ /* no bends between two splines. */ - if ( !tp && ( !fp || t > 0.5 ) && - topd->colinear && &other->to->next != NULL ) { Change it to `topd->next != NULL` rather than deleting it. That's the likely original intention and it can't hurt. > @@ -2378,8 +2377,7 @@ return( 0 ); } if ( tp ) t_needs_recalc = true; } - if ( !fp && ( !fp || t < 0.5 ) && - frompd->colinear && &other->from->prev != NULL ) { + if ( !fp && ( !fp || t < 0.5 ) && frompd->colinear ) { Make this one `frompt->prev != NULL`.

[83/93] Building C object fontforge/CMakeFiles/fontforge.dir/tottf.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/tottf.c:4507:45: warning: variable 'cnt' set but not used [-Wunused-but-set-variable] 4507 | int curseg=0, segcnt, segmax=SEGMAXINC, cnt=0, mapcnt=0; | ^ /home/nabijaczleweli/uwu/fontforge/fontforge/tottf.c:4704:9: warning: variable 'chk' set but not used [-Wunused-but-set-variable] 4704 | int chk=0; | ^ 2 warnings generated.

[86/93] Building C object fontforgeexe/CMakeFiles/fontforgeexe.dir/charview.c.o /home/nabijaczleweli/uwu/fontforge/fontforgeexe/charview.c:1501:9: warning: variable 'currentSplineCounter' set but not used [-Wunused-but-set-variable] 1501 | int currentSplineCounter = 0; | ^ 1 warning generated.

/home/nabijaczleweli/uwu/fontforge/fontforgeexe/problems.c:817:14: warning: using integer absolute value function 'abs' when argument is of floating point type [-Wabsolute-value] 817 | } else if ( abs(yoff)>abs(xoff) ) | ^ /home/nabijaczleweli/uwu/fontforge/fontforgeexe/problems.c:817:14: note: use function 'fabs' instead 817 | } else if ( abs(yoff)>abs(xoff) ) | ^~~ | fabs /home/nabijaczleweli/uwu/fontforge/fontforgeexe/problems.c:817:24: warning: using integer absolute value function 'abs' when argument is of floating point type [-Wabsolute-value] 817 | } else if ( abs(yoff)>abs(xoff) ) | ^ /home/nabijaczleweli/uwu/fontforge/fontforgeexe/problems.c:817:24: note: use function 'fabs' instead 817 | } else if ( abs(yoff)>abs(xoff) ) | ^~~ | fabs

skef · 2024-11-11T04:25:27Z

@iorsh this tentatively looks good to me. Any thoughts?

iorsh

Looks good to me

nabijaczleweli added 20 commits November 8, 2024 18:29

scstyles: remove unused variable

64f1c7a

[129/280] Building C object fontforge/CMakeFiles/fontforge.dir/scstyles.c.o /home/nabijaczleweli/uwu/fontforge/fontforge/scstyles.c:5383:9: warning: variable 'i' set but not used [-Wunused-but-set-variable] 5383 | int i; | ^ 1 warning generated.

exe/fontview: remove useless sllistcheck() body

f932100

Found by clang self-assignment warning

nabijaczleweli changed the title ~~Warning rollup from clang trunk~~ Warning rollup (probably some hidden bugs!) from clang trunk Nov 8, 2024

skef reviewed Nov 11, 2024

View reviewed changes

nabijaczleweli added 4 commits November 11, 2024 05:17

iorsh approved these changes Nov 11, 2024

View reviewed changes

skef merged commit 9af60ed into fontforge:master Nov 12, 2024

tuffnatty mentioned this pull request Nov 14, 2025

variable 'svg' is used uninitialized #5273

Closed

tuffnatty mentioned this pull request Nov 14, 2025

variable 'colorp' is uninitialized #5274

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warning rollup (probably some hidden bugs!) from clang trunk#5492

Warning rollup (probably some hidden bugs!) from clang trunk#5492
skef merged 24 commits intofontforge:masterfrom
nabijaczleweli:master

nabijaczleweli commented Nov 8, 2024

Uh oh!

Uh oh!

Uh oh!

skef Nov 11, 2024

Uh oh!

nabijaczleweli Nov 11, 2024 •

edited

Loading

Uh oh!

skef Nov 11, 2024

Uh oh!

skef commented Nov 11, 2024

Uh oh!

iorsh left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nabijaczleweli commented Nov 8, 2024

Type of change

Uh oh!

Uh oh!

Uh oh!

skef Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

nabijaczleweli Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skef Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

skef commented Nov 11, 2024

Uh oh!

iorsh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nabijaczleweli Nov 11, 2024 •

edited

Loading