<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Aleksei,<div><br></div><div>Thanks for sending in the patch and adding a lot of improvements to the checker.</div><div><br></div><div>Here are some higher level issues I’ve noticed:</div><div><br></div><div> - We strongly prefer not to bifurcate the state whenever possible since it might introduce a lot of performance overhead (you essentially double the work from that point on). The general strategy is to check if the symbol is in valid state before reporting an error. In some cases, you might need to associate another symbol that denotes success or failure of an operation with the primary tracked symbol (in case they are different). Part of the problem could be caused by StreamChecker performing eval::Call instead of registering for these:</div><div> void checkPostCall(const CallEvent &Call, CheckerContext &C) const;</div> void checkPreCall(const CallEvent &Call, CheckerContext &C) const;<br><br><div> - How do you deal with pointer escape? Looks like StreamChecker does not register for that callback. (See SimpleStreamChecker.cpp for how it should be used.)</div><div><br></div><div> - As I’ve mentioned in one of our previous communications, the improvements to the StreamChecker should be based on SimpleStreamChecker since there are a few subtle issues with the old checker. I would advise to replace the existing StreamChecker with SimpleStreamChecker as the base and add the improvements you’ve added to Stream Checker to it one by one. (Also, this patch is quite large so that would be a way to break it up a bit.)</div><div><br></div><div>Sorry it took us a while to start on the review!</div><div><br></div><div>Cheers,</div><div>Anna.</div><div><br></div><div><br><div><blockquote type="cite"><div>On Apr 28, 2014, at 9:35 PM, Aleksei Sidorin <<a href="mailto:a.sidorin@samsung.com">a.sidorin@samsung.com</a>> wrote:</div><br class="Apple-interchange-newline"><div>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div text="#000000" bgcolor="#FFFFFF">
Hello,<br>
<br>
This is an another version of patch for StreamChecker.<br>
Improvements:<br>
1. open/close Unix API support<br>
2. BugVisitor with open/close messages (based on MallocChecker's
one)<br>
And a question: is it OK to use StringSwitch for function names in
evalCall or it is too slow?<br>
<br></div></div></blockquote> </div><div>We prefer to use matching on identifiers instead of string compare. You can initialize them once for all functions you are about. You can use SimpleStreamChecker.cpp for an example.<br><br><blockquote type="cite"><div><div text="#000000" bgcolor="#FFFFFF">
<pre wrap=""><cite>Message: 1
Date: Fri, 07 Jun 2013 16:05:19 +0400
From: Alexey Sidorin </cite><cite><a class="moz-txt-link-rfc2396E" href="mailto:a.sidorin@samsung.com"><a.sidorin@samsung.com></a></cite><cite>
To: </cite><cite><a class="moz-txt-link-abbreviated" href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a></cite><cite>
Subject: [PATCH] CSA's StreamChecker - additional functionality and
fix
Message-ID: </cite><cite><a class="moz-txt-link-rfc2396E" href="mailto:51B1CC7F.4020202@samsung.com"><51B1CC7F.4020202@samsung.com></a></cite><cite>
Content-Type: text/plain; charset="utf-8"; Format="flowed"
Hi,
I wrote a patch for Clang Static Analyzer's StreamChecker. This patch:
1. adds a check for descriptor access (read/write) after it being closed
2. adds support of directory operations (opendir/closedir and some other)
3. fixes issue in double close check: descriptor that was not tracked
before was not marked as closed while calling a close function
4. adds fprintf and fscanf functions to track.
Is it OK to commit or have I done something wrong?</cite></pre>
<br>
<br>
<br>
<pre class="moz-signature" cols="72">--
Best regards,
Aleksei Sidorin
Software Engineer,
IMSWL-IMCG, SRR, Samsung Electronics
</pre>
</div>
<span><StreamCheckerv2.patch></span>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></div></blockquote></div><br></div></body></html>