

Even DJB's code is buggy. (And some others too.) - dgn
http://www.google.com/codesearch?hl=en&lr=&q=%22if+%28errno+%3D+%22&sbtn=Search

======
bayareaguy
DJB only sets errno_intr here in error.c

    
    
      int error_intr =
      #ifdef EINTR
      EINTR;
      #else
      -1;
      #endif
    

The (errno = error_intr) happens in this block:

    
    
       default:
          for (;;) {
    	r = read(fdfile,inbuf,sizeof inbuf);
    	if (r == -1) {
    	  if (errno = error_intr) continue;
    	  _exit(23);
    	}
    	if (r == 0)
    	  break;
    	if (!fetch_ascii)
    	  substdio_put(&ss,inbuf,r);
    	else
    	  for (i = 0;i < r;++i) {
    	    if (inbuf[i] == '\n')
    	      substdio_put(&ss,"\r",1);
    	    substdio_put(&ss,inbuf + i,1);
    	  }
          }
          break;
    

Like a lot of DJB code, it's hard to tell exactly what he's trying to do. He
could be trying to deal with some weird platform. Where EINTR is defined to
something other than 0 that code will cause the read() to be repeated whenever
it returns -1 and when the code eventually gets back to the caller errno will
be left set to EINTR. But on platforms where EINTR is 0, a read error will
exit.

~~~
pc
You're right that this could theoretically be the intent -- but it's very
unlikely that it's necessary: "The value of errno is zero at program startup"
- ISO/IEC 9899:TC2. If EINTR was 0, there'd be no way of detecting it.

------
snprbob86
Yet another example of why you should always compile with warnings as errors
and warnings set as high as you can tolerate. Oh, and if you have a static
type system, you might as well use it by preventing implicit casts to bool.

------
henning
I'm assuming this bug is an instance of a more general one, where people write
if(... = ...) when they meant to use == instead?

Hence, here, error-handling code will be improperly invoked irrespective of
whether they get a given error code or not.

~~~
davidw
> I'm assuming this bug is an instance of a more general one

Anyone want to work on a regexp for the more general condition?

~~~
brianr
Most of these don't look like bugs, but here's a search to match "if (... = ":

[http://www.google.com/codesearch?hl=en&lr=&q=if\\+\](http://www.google.com/codesearch?hl=en&lr=&q=if\\+\\)(\w%2B\\+%3D\\+&sbtn=Search

Can anyone think of a better way to match the subset that are more likely to
be bugs?

~~~
alecco
[http://www.google.com/codesearch?hl=en&lr=&q=if\\+\](http://www.google.com/codesearch?hl=en&lr=&q=if\\+\\)(\w%2B\\+%3D\\+[^(\\.]%2B\\+%3F\\)&sbtn=Search

That takes out parentheses (for function calls) and structure member
assignment because it might be two statements in one line (ugly and error
prone as output will be exactly the same for a = b.c; if(a) {}; vs. if (a=b.c)

------
Lagged2Death
I have for years made it a habit to write "if (CONSTANT == variable)" rather
than "if (variable == CONSTANT)" so that if (oh, let's be honest -- _when_ ) I
accidentally use "=" instead of "==" I'll get a big nasty compiler error.

Some people have commented that it "looks kinda backwards," and I'm not sure
what to make of that. My "backwards" way is perfectly correct, C-wise and
math-wise, it doesn't make the code hard to understand, and it solves a real
problem.

But I appreciate that it's important not to upset the other people on your
team willy-nilly. So it's a bit of a quandry.

~~~
altano
Other things to prevent this error: 1) Don't ever use assignment in a
conditional expression, even on purpose. 2) Aggressively declare everything as
const, even scalar input parameters to your function. 3) Use code analysis
tools that treat this as a warning, which you should be treating as an error.

------
jgalvez
I'm sorry, who (or what) is DJB and what codebase are you referring to
exactly? Also, what is the bug there? Can anyone educate me?

~~~
csl
Daniel Bernstein (qmail, djbdns, etc): <http://cr.yp.to>

The bug searched for is using the assignment operator = instead of the
equality operator ==.

~~~
jgalvez
Ha, thanks! I didn't notice '=' was wrong there, part of me thinks that can
actually be common in C (assigning the result of a condition to be further
used within the block).

~~~
ConradHex
It can be intentional if the right side is a function call.

But something like:

if (errno = 0) { blah(); }

is almost certainly a bug.

------
maurycy
Despite ugly style, this is not a bug actually, assuming that error_intr is
meaningful, and continue should be called only if error_intr is false:

if (errno = error_intr) continue;

~~~
pc
error_intr is meaningful, but it's defined as the value of EINTR, which can
never be zero -- so this is a bug.

------
aaronsw
This was caught here:

<http://www.nabble.com/Bug-in-fetch.c:doit--td1143190.html>

------
jbert
Some of those look like instances of a local variable called errno being used
as a return code.

That's OK, it'll just shadow the global errno in that func, no?

------
bumbledraven
I don't remember hearing DJB claim that his code was free of bugs, only that
it was free of security holes.

