Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-19 Thread Alexei Podtelezhnikov
Indeed, the loop body is exited and reentered in each iteration so the scope is lost each time. It is only thanks to the reuse of stack that the code worked. The last fix by Werner is correct. I would initialize to -1 though for consistency or not initialize at all. On Thu, Jun 19, 2014 at 12:40

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-18 Thread Behdad Esfahbod
On 14-06-18 01:05 AM, Werner LEMBERG wrote: It's not *that* mysterious! The code looks like this. bool flag = 0; while { int a; ... if (flag) a = 1; else a = 0; ... flag = 1; } It doesn't. It looks like this: while { int

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-17 Thread Werner LEMBERG
Oops! You are right. Thanks for catching this; it's now reverted. Arg!!! This thing (aflatin.c:571) got wrongly fixed again. Please revert with an inline comment so that people think rather than trust clang. *blush* How embarassing. This time it's the MSVC compiler. I've fixed it in

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-17 Thread Sean McBride
On Mon, 16 Jun 2014 20:46:44 -0400, Alexei Podtelezhnikov said: Arg... I am not sure if the patch is even correct. Aren't we resetting the variable in each loop cycle? This wasn't the case in the original code where the values reused, was it? Oops! You are right. Thanks for

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-17 Thread Werner LEMBERG
I submit that there is a code smell here. The code is funky enough to trick two different tools, humans, and human reviewers. While it may be 'correct', it's probably too complicated and should be refactored in some way. Yep. I did that with my last commit. Werner

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-17 Thread Alexei Podtelezhnikov
On Tue, Jun 17, 2014 at 10:29 AM, Sean McBride s...@rogue-research.com wrote: On Mon, 16 Jun 2014 20:46:44 -0400, Alexei Podtelezhnikov said: Arg... I am not sure if the patch is even correct. Aren't we resetting the variable in each loop cycle? This wasn't the case in the

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-17 Thread Werner LEMBERG
I submit that there is a code smell here. The code is funky enough to trick two different tools, humans, and human reviewers. While it may be 'correct', it's probably too complicated and should be refactored in some way. It is a hack of code! I'll try to understand it fully and rework.

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-06-16 Thread Alexei Podtelezhnikov
On Wed, Mar 19, 2014 at 2:36 AM, Werner LEMBERG w...@gnu.org wrote: Arg... I am not sure if the patch is even correct. Aren't we resetting the variable in each loop cycle? This wasn't the case in the original code where the values reused, was it? Oops! You are right. Thanks for

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-19 Thread Werner LEMBERG
Arg... I am not sure if the patch is even correct. Aren't we resetting the variable in each loop cycle? This wasn't the case in the original code where the values reused, was it? Oops! You are right. Thanks for catching this; it's now reverted. Werner

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-18 Thread Werner LEMBERG
The patch 0004 is false positive. The variable 'hit' is set to zero when entering the loop so 'p_last' and 'p_first' are assigned almost immediately. Thanks for the analysis. However, initializing doesn't hurt, and pacifying one of the most used compilers is probably a good thing.

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-18 Thread Werner LEMBERG
Attached are 3 more patches, this time to fix compiler warnings (not static analyzer warnings). Thanks. I've applied most of them (some with changes). However, these three I don't understand (in file src/psaux/psauxmod.c): - FT_CALLBACK_TABLE_DEF + static const AFM_Parser_FuncsRec

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-18 Thread Alexei Podtelezhnikov
On Tue, Mar 18, 2014 at 2:37 AM, Werner LEMBERG w...@gnu.org wrote: The patch 0004 is false positive. The variable 'hit' is set to zero when entering the loop so 'p_last' and 'p_first' are assigned almost immediately. Thanks for the analysis. However, initializing doesn't hurt, and

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-18 Thread Alexei Podtelezhnikov
On Tue, Mar 18, 2014 at 2:37 AM, Werner LEMBERG w...@gnu.org wrote: The patch 0004 is false positive. The variable 'hit' is set to zero when entering the loop so 'p_last' and 'p_first' are assigned almost immediately. Thanks for the analysis. However, initializing doesn't hurt, and

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-18 Thread Sean McBride
On Tue, 18 Mar 2014 17:07:08 -0400, Alexei Podtelezhnikov said: The patch 0004 is false positive. The variable 'hit' is set to zero when entering the loop so 'p_last' and 'p_first' are assigned almost immediately. Thanks for the analysis. However, initializing doesn't hurt, and

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-14 Thread Sean McBride
Werner, Attached are 3 more patches, this time to fix compiler warnings (not static analyzer warnings). None of them are serious. The biggest fixes most -Wmissing-variable-declarations warnings, where some functions that are private were not declared static. Cheers, --

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-14 Thread Alexei Podtelezhnikov
On Fri, Mar 14, 2014 at 11:18 AM, Sean McBride s...@rogue-research.comwrote: Attached are 3 more patches, this time to fix compiler warnings (not static analyzer warnings). None of them are serious. The biggest fixes most -Wmissing-variable-declarations warnings, where some functions that

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-08 Thread Werner LEMBERG
With Xcode 4.6.3, there is only one warning left. Patch attached. Applied, thanks. I'm a little less confident in this one due to that 'volatile' being there Well, the `volatile' is for the longjmp call, which happens after this error check; thus there shouldn't be a problem with your

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-07 Thread Sean McBride
On Thu, 6 Mar 2014 14:55:33 +0100, Werner LEMBERG said: Attached is a more straightforward fix. Applied, thanks! Thanks. With Xcode 4.6.3, there is only one warning left. Patch attached. I'm a little less confident in this one due to that 'volatile' being there Cheers, --

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-06 Thread Werner LEMBERG
Attached is a more straightforward fix. Applied, thanks! Werner ___ Freetype-devel mailing list Freetype-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/freetype-devel

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-05 Thread Sean McBride
On Tue, 4 Mar 2014 04:50:06 +0100, Werner LEMBERG said: Here's another 2 patches to fix most of the remaining warnings. Thanks, I've applied the first one (and sorry for forgetting git commit's `--author' flag). No worries! However, I strongly dislike the second one. No problem, I separated

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-03 Thread Sean McBride
On Sat, 8 Feb 2014 13:56:37 +0100, Werner LEMBERG said: Attached is a work-in-progress patch that fixes several clang static analyzer and compiler warnings. It's not in a state were it could be immediately applied, because for some warnings I was not sure of the best solution, and so just

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-03-03 Thread Werner LEMBERG
Here's another 2 patches to fix most of the remaining warnings. Thanks, I've applied the first one (and sorry for forgetting git commit's `--author' flag). However, I strongly dislike the second one. IMHO, this is wrong analysis of clang (BTW, what exactly does it report?). The declaration

Re: [ft-devel] WIP PATCH: clang static analyzer and warning fixes

2014-02-08 Thread Werner LEMBERG
Attached is a work-in-progress patch that fixes several clang static analyzer and compiler warnings. It's not in a state were it could be immediately applied, because for some warnings I was not sure of the best solution, and so just added a comment explaining. Also, I don't know the