https://bugzilla.redhat.com/show_bug.cgi?id=1367598 --- Comment #2 from Jerry James <loganjerry@xxxxxxxxx> --- Thanks for the review, Paulo. (In reply to Paulo Andrade from comment #1) > It is not required to have: > BuildRequires: gcc Actually, it is now. See: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/message/XGXCCAJNEOIEN3KK6TN2657LSIGZGB3N/ > This looks suspecting, as it means it will loop only once: > ./src/randschr.c:238:10: warning: this 'if' clause does not guard... > [-Wmisleading-indentation] Good catch! I will add braces to the warning patch to fix that. > || and && have different precedence, so this code is also suspecting: > ./src/randschr.c:391:43: warning: suggest parentheses around '&&' within > '||' [-Wparentheses] > lowestOrder == curOrder && levelLowestOrder > finalLevel) > ) { > ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ That one I think is okay. It appears to me that the && really should be within ||, so it is parsed in the correct order. But ... it's easy to patch, so I will add parentheses there. > Another case of suspicious code: > ./src/ccent.c: In function 'nextBasePointEltCent': > ./src/ccent.c:90:24: warning: suggest parentheses around '+' inside '<<' > [-Wparentheses] > priority = 2000000000ul - (unsigned long) > MIN(cycleLen[pt],1000) << 20 > > ~~ > + cSize; > ^~~~~~~ This one worries me. I'm not sure if the + is supposed to be inside or outside of the <<, so I am reluctant to touch it. I think I'd better check with upstream before I do anything here, unless you think you can see where the parentheses should go. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx