3 ms·

Ah, okay. Personally I'd still want to do the check before, because someone could still be adding something in-between the assignment and the check, but if tha

by faho 5y ago

Ah, okay. Personally I'd still want to do the check before, because someone could still be adding something in-between the assignment and the check, but if that's the order it's resolved in it seems to be working now. (I'm guessing they declare and assign the variable first because of C version constraints - wasn't that something that changed in C99?)

aaronmdjones 5y ago
Yes; it's C99 that let you declare variables anywhere in a function, rather than only before any statements. So it could be rewritten as: if (fd == -1) return; if (p == -1 || (u_int)p >= npfd) fatal_f("channel %d: bad pfd %d (max %u)", c->self, p, npfd); struct pollfd *pfd = &pfds[p]; dump_channel_poll(__func__, what, c, p, pfd); This is how I would have written it (I exclusively write C99), but the OpenSSH developers target a lot more platforms that may not have reliable C99 compilers, and are understandably reticent to rely upon the GNU extensions to C89 that would also let them do this.
- mdaniel 5y ago
  For consideration, "declare variable" is just that; one could still choose to only assign to it after having done the validation (assuming fatal_f causes control flow termination) struct pollfd *pfd; if (p == -1 || (u_int)p >= npfd) fatal_f("channel %d: bad pfd %d (max %u)", c->self, p, npfd); pfd = &pfds[p]; right?
  - fullstop 5y ago
    A good compiler would produce the same code in both cases.
- nybble41 5y ago
  You don't need C99 for this, just some extra braces: if (fd == -1) return; if (p == -1 || (u_int)p >= npfd) fatal_f("channel %d: bad pfd %d (max %u)", c->self, p, npfd); { struct pollfd *pfd = &pfds[p]; dump_channel_poll(__func__, what, c, p, pfd); } It's perfectly valid C89 to declare a variable at the start of a nested compound statement no matter where it appears in the outer block, so in practice you can declare variables wherever you want so long as you don't mind the extra clutter.

by faho 5y ago

aaronmdjones 5y ago

Yes; it's C99 that let you declare variables anywhere in a function, rather than only before any statements. So it could be rewritten as: if (fd == -1) return; if (p == -1 || (u_int)p >= npfd) fatal_f("channel %d: bad pfd %d (max %u)", c->self, p, npfd); struct pollfd *pfd = &pfds[p]; dump_channel_poll(__func__, what, c, p, pfd); This is how I would have written it (I exclusively write C99), but the OpenSSH developers target a lot more platforms that may not have reliable C99 compilers, and are understandably reticent to rely upon the GNU extensions to C89 that would also let them do this.

mdaniel 5y ago
For consideration, "declare variable" is just that; one could still choose to only assign to it after having done the validation (assuming fatal_f causes control flow termination) struct pollfd *pfd; if (p == -1 || (u_int)p >= npfd) fatal_f("channel %d: bad pfd %d (max %u)", c->self, p, npfd); pfd = &pfds[p]; right?
- fullstop 5y ago
  A good compiler would produce the same code in both cases.
nybble41 5y ago
You don't need C99 for this, just some extra braces: if (fd == -1) return; if (p == -1 || (u_int)p >= npfd) fatal_f("channel %d: bad pfd %d (max %u)", c->self, p, npfd); { struct pollfd *pfd = &pfds[p]; dump_channel_poll(__func__, what, c, p, pfd); } It's perfectly valid C89 to declare a variable at the start of a nested compound statement no matter where it appears in the outer block, so in practice you can declare variables wherever you want so long as you don't mind the extra clutter.