6 ms·

There are so many things in here that tempt me to comment about, so here goes: 1) For me, this is a prime example of why I personally like programming environm

by pilif 12y ago

There are so many things in here that tempt me to comment about, so here goes: 1) For me, this is a prime example of why I personally like programming environments with exceptions. If libnih could throw an exception (I know it can't), then they could do that which would allow the caller to at least deal with the exception and not bring the system down. If they don't handle the exception, well, we're were we are today, but as it stands now, fixing this will require somebody to actually patch libnih. Yes. libnih could also handle that error by returning an error code itself, but the library developers clearly didn't want to bother with that in other callers of the affected function. By using exceptions, for the same amount of work it took to add the assertion they could also have at least provided the option for the machine to not go down. Also, I do understand the reservations against exceptions, but stuff like this is what makes me personally prefer having exceptions over not having them. 2) I read some passive aggressive "this is what happens if your init system is too complicated" assertions between the lines. Being able to quickly change something in /etc/init and then have the system react to that is actually very convenient (and a must if you are pid 1 and don't want to force restarts on users). Yes, the system is not prepared to handle 10Ks of init scripts changing, but if you're root (which you have to be to trigger this), there are way more convenient (and quicker!) ways to bring down a machine (shutdown -h being one of them). Just removing a convenient feature because of some risk that the feature could possibly be abused by an admin IMHO isn't the right thing to do. 3) I agree with not accepting the patch. You don't (ever! ever!) fix a problem by ignoring it somewhere down the stack. You also don't call exit() or an equivalent in a library either of course :-). The correct fix would be to remove the assertion, to return an error code and to fix all call sites (good luck with the public API that you've just now changed). Or to throw an exception which brings us back to point 1. I'm not complaining, btw: Stuff has happened (great analysis of the issue, btw. Much appreciated as it allowed me to completely understand the issue and be able to write this comment without having to do the analysis myself). I see why and I also understand that fixing it isn't that easy. Software is complicated. The one thing that I'm heavily disagreeing though is above point 2). Being able to just edit a file is way more convenient than also having to restart some daemon (especially if that has PID 1). The only fix from upstarts perspective would be to forego the usage of libnih (where the bug lives), but that would mean a lot of additional maintenance work in order to protect against a totally theoretical issue as this bug requires root rights to use.

ars 12y ago
I fail to see how an exception vs an error code makes any difference whatsoever. The whole problem is that the library doesn't notice the error. If you don't notice an error you are not going to use an exception either. Not to mention that an exception by default terminates the program, which is exactly what you don't want to do.
- pilif 12y ago
  The library noticed the error and was (effectively) calling exit(). If the library was at liberty to throw, they would not have to change any internal callers and would get the same feature for them (not having to deal with errors anywhere) but they would still give the external caller a chance not to terminate. Yes. By default, a caller would terminate, but it would get a chance not to in a way that doesn't involve changing the libraries public interface.
  - willvarfar 12y ago
    An assert is what a programmer uses when they think something won't/can't ever happen. So the authors did not realise inotify could legitimately return an error. If they'd thought it could return an error, they'd return the error code themselves or recover gracefully or whatever.
  - vidarh 12y ago
    The library can do the equivalent by offering a new interface to register an error callback. Even without that, callers do have a way of preventing termination: Fork.
  - adamtj 12y ago
    Exceptions are not a silver bullet. One can't simply turn an assert into an exception and expect things to work. That's a great way to add new and tricky bugs unrelated to the original issue. If an exception crosses a stack frame that isn't expecting one, things could be left in an inconsistent state. Invariants may vary, constraints may be unconstrained. To retrofit exceptions into the existing code at this point would be very difficult, as you would have to audit every caller of the function in question, a nd the callersof those, and on and on... Imagine problems like Apple's "goto fail" SSL bug, but more obscure and harder to find. To have used exceptions from the beginning would require an amount of work roughly equivalent to returning error codes. They aren't fundamentally different from error codes in that way. Well, unless you're a cowboy who ignores errors. Exceptions wouldn't have solved anything here.

by pilif 12y ago

ars 12y ago

I fail to see how an exception vs an error code makes any difference whatsoever. The whole problem is that the library doesn't notice the error. If you don't notice an error you are not going to use an exception either. Not to mention that an exception by default terminates the program, which is exactly what you don't want to do.

pilif 12y ago
The library noticed the error and was (effectively) calling exit(). If the library was at liberty to throw, they would not have to change any internal callers and would get the same feature for them (not having to deal with errors anywhere) but they would still give the external caller a chance not to terminate. Yes. By default, a caller would terminate, but it would get a chance not to in a way that doesn't involve changing the libraries public interface.
- willvarfar 12y ago
  An assert is what a programmer uses when they think something won't/can't ever happen. So the authors did not realise inotify could legitimately return an error. If they'd thought it could return an error, they'd return the error code themselves or recover gracefully or whatever.
- vidarh 12y ago
  The library can do the equivalent by offering a new interface to register an error callback. Even without that, callers do have a way of preventing termination: Fork.
- adamtj 12y ago
  Exceptions are not a silver bullet. One can't simply turn an assert into an exception and expect things to work. That's a great way to add new and tricky bugs unrelated to the original issue. If an exception crosses a stack frame that isn't expecting one, things could be left in an inconsistent state. Invariants may vary, constraints may be unconstrained. To retrofit exceptions into the existing code at this point would be very difficult, as you would have to audit every caller of the function in question, a nd the callersof those, and on and on... Imagine problems like Apple's "goto fail" SSL bug, but more obscure and harder to find. To have used exceptions from the beginning would require an amount of work roughly equivalent to returning error codes. They aren't fundamentally different from error codes in that way. Well, unless you're a cowboy who ignores errors. Exceptions wouldn't have solved anything here.