-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements to C code that can improvement robustness #455
Comments
Hi Niranjan, Thanks for finding these issues. Your tool sounds very useful! A pull request would be much appreciated. Cheers, |
Btw, what did you mean by "a considerably high number of stars"? :) |
Hi Steve, Thanks. Yes, I can submit a pull request. Sorry my phrase was not precise. :) By stars, I meant the GitHub stars - we have analyzed more than 6000 repositories that use C as the primary language and have more than 100 GitHub stars, and 2.3K stars for this repository are higher than several other repositories. |
hi Steve, Sorry for the delay, just to update - I am working on sending a pull request. Please don't close this issue yet. :) Thanks |
Rewrote UArray comparison functions to use boolean type instead of int type to resolve any ambiguity. Modified signatures of the corresponding functions. Ran tests and checked that they pass: $ LD_LIBRARY_PATH=`pwd`/install/lib/ ../install/bin/io ../libs/iovm/tests/correctness/run.io ...................................................................... ...................................................................... ...................................................................... .................... ---------------------------------------------------------------------- Ran 230 tests in 3.0030000000000001s OK run
Thanks for the pull request Niranjan. This is a nice improvement. |
Hi,
I work at Intel, and we have developed a tool that detects anomalous programming language expressions that can possibly lead to bugs. We scanned the code repository for this project as it has considerably high number of stars!
We found a couple of places where the code/expressions are confusing and seem to implement the logic in a rather convoluted manner. We think that the expressions could be rewritten to capture the logic accurately and precisely.
Case 1) libs/basekit/source/UArray.c:999
The expression
if (UArray_greaterThan_(self, other) | UArray_equals_(self, other))
looks confusing because the intended logic of logical OR seems to be implemented using bitwise OR on integers. While this is technically correct, it is a bit confusing for someone reading this code for the first time. It looks like the better approach would be: 1) one option would be to usebool
type from C99, and 2) another option would be to use enum type for true and false.Case 2) extras/win32vc2005/freeglut-2.2.0/src/freeglut_structure.c#L624 and L626
It looks like expressions
if( ln = (SFG_Node *)node->Next )
andif( ln = (SFG_Node *)node->Prev )
in line 624 and 626 resp., are missing parenthesis around the assignment. The expressions, similar toif( (node->Next = next) )
, in other parts of the code correctly add those parenthesis.Any thoughts on the findings? If this looks acceptable, I'm happy to send a pull request with the changes.
Thanks.
The text was updated successfully, but these errors were encountered: