Improvements to C code that can improvement robustness

[nhasabni]

Hi,

Sorry for not using the form to submit. This is not a bug report, but a rather report of our findings obtained from scanning the repository.

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) master/machine/util/vsnprintf.c:1099

The expression else if (ch >= 'f') { /* 'f' or 'g' */ seems to check if ch is 'f' or 'g', but the check is rather a bit broad? We believe it is better to use else if (ch == 'f' || ch == 'g') directly as per the comment (if the comment is correct).

Case 2) build/libraries/oniguruma/st.c#L538

The expression if ( g = h & 0xF0000000 ) is missing parenthesis as the assignment is used as a truth value. Similar to -Wall option from compilers, our tool is also able to capture this issue. We believe the parenthesis around the expression would convey the intended semantics more clearly.

Case 3) build/libraries/zlib/examples/gzlog.c

Code in lines 956, 967, and 1017 uses pattern of type shown below. This pattern looks confusing because exact intention is a bit convoluted. Because bitwise OR in C language does not go through short-circuit evaluation, close(fd) will be called irrespective of value of ret. If this is intention, then using boolean types could get rid of the confusion.

ret = (size_t)write(fd, data, len) != len;
if (ret | close(fd))
  return -1;

Any thoughts on the findings?

Thanks.