6 ms·

Are you arguing that we should leave known crash bugs in the code?

by larsrc 6y ago

aidos 6y ago
It’s not a bug, in this case. It depends on the behaviour the library is specified to handle, but if you ask me to remove a specific item from a list, and that item isn’t in the list, what should I do? I can fail so you can catch it, or if you don’t catch it, you’ll at least see exactly what went wrong. You expected the item to be in the list and it wasn’t. Your preconditions were wrong from the beginning and you have an (unknown) bug in the code. The alternative is that I can’t find the item and I silently suck it up and don’t notify you. If that’s the api we’ve all agreed on, that’s fine. It was on you to check for the thing in the list first and then remove it. But it’s a bit weird if you think about it - now you need to traverse the list twice. Once to check for the existence (and fail out yourself) and once to remove.
- christophilus 6y ago
  I think a better approach is to return a bool or something like that, rather than crashing. But I agree that silently handling this case is the worst of the options.
adrianN 6y ago
I'm arguing that we should use asserts or similar defensive coding mechanisms to ensure that preconditions are met instead of merrily continuing to run our program with cascading error effects.
- BoorishBears 6y ago
  I've had this argument a ton with people used to new high level languages that make null safety "too easy" like: next?.doThing() Where doThing is never called if next is null. Languages that do this use "scary" operators to crash on null: next!!.doThing() And it's drilled into people's heads the latter is a Bad Thing (tm) - You really need to consider context in null handling. Imagine an app that alerts a nurse when the patients heartbeat is out of range. In an application where the UI context might have been closed out, it's common to see someUiContext?.showAlert() But what actually happens if the context is gone? It's better to crash and have part of your startup procedure be communicating that a crash occurred, and the doctor should check that something went wrong, than silently continuing. - The problem is when you tell people this, the kneejerk reaction is always "are you saying we intentionally add crashes"! Because safe null handling was specifically added to avoid the situation where that crashes... (For example, if the app was a news reader and the alert was "Article failed to load", you wouldn't want the app to crash just because the user left a certain page before the alert was shown) But I think the pendulum has swung too far at this point, people are so used to just sweeping nulls under the rug, and it's not great for finding issues
- veets 6y ago
  Yes - and that is the point the OP you responded to is making. This is not a generic library function. It used in a specific setting where preconditions exist and presumably are checked _prior_ to calling this code. Sure you could make the argument that we don't _know_ they are being checked, but it's a pointless discussion. Who cares? _If_ preconditions are met, this code is safe, if they aren't, it's not safe. Since we don't know one way or the other, there's no point in discussing further. The kernel developers know their stuff...
  - adrianN 6y ago
    Imo every piece of code (for reasonable definitions of "piece") is supposed to check its own preconditions and not rely on the caller to check them.
    - veets 6y ago
      This is nice in theory but a bad idea in practice (as a blanket statement, I am all for checking preconditions in general). An easy example is binary search, which I think is a reasonable "piece" of code by your definition. One should never check its preconditions _inside_ the binary search function (that the list of elements being searched is partitioned by the search predicate). Checking the precondition is O(n) while the algorithm is O(lg n).
      - adrianN 6y ago
        You're right, but checking for null is not terribly expensive compared to iterating over a linked list.
        HunterWare 6y ago
        The thing is that you aren’t really advocating for just that... Since this is pretty general support library code, the result of this philosophy is defensive checks in all support functions and that means checks every spinlock, mutex/semaphore, and god knows how many other places... everything really. We also would want these checks at multiple levels since we shouldn’t trust that other people checked things. This would have a significant effect on performance and many would prefer their system not to run however-many percent slower. All to avoid a possible issue that can’t be very large or systems would be kernel panic’ing all over the place. From a black box, zero-knowledge, perspective it’s maybe worth remembering that code built this way successfully runs mission critical systems all over the world every day. Thoughtful people would have switched to other (slower, more pedantic) systems if doing things differently was a real overall life improvement. People have had the choice, there are plenty of OS’s out there... some far more formal/pedantic. Linux wasn’t the safe choice back in the day, it was just better by several important metrics consistently over time. Perhaps these insanely experienced kernel devs know what they are doing.
- andrepd 6y ago
  Asserts have run-time cost. What you need is to ensure ensure those preconditions are met in the first place, statically, by formal methods for example.
  - throwaway_pdp09 6y ago
    Which is ideal, but generally beyond reach for now. So assertions it is.
banachtarski 6y ago
What you are espousing is known as “defensive programming” which is very much a bad thing (tm).
- adrianN 6y ago
  I'm interested in hearing more about why defensive programming is bad. Do you have some links for me?
- stuff4ben 6y ago
  Clearly you've never worked with contract developers whose only purpose is to write a crap ton of code to get a "feature" out that crashes as soon as it encounters the real world. As they say, "doveryay, no proveryay" and it's served me well.
  - hyperdimension 6y ago
    > As they say, "doveryay, no proveryay" I'm sorry if this might be common knowledge, but I've honestly never heard this and can't understand it. Is it saying that it's better to do something than to (formally) prove it, like the opposite of (I think) Knuth's famous quote?
    - DanBC 6y ago
      "trust, but verify". https://en.wikipedia.org/wiki/Trust,_but_verify https://en.wikipedia.org/wiki/Trust,_but_verify

Are you arguing that we should leave known crash bugs in the code?

by larsrc 6y ago

Are you arguing that we should leave known crash bugs in the code?

It’s not a bug, in this case. It depends on the behaviour the library is specified to handle, but if you ask me to remove a specific item from a list, and that item isn’t in the list, what should I do? I can fail so you can catch it, or if you don’t catch it, you’ll at least see exactly what went wrong. You expected the item to be in the list and it wasn’t. Your preconditions were wrong from the beginning and you have an (unknown) bug in the code. The alternative is that I can’t find the item and I silently suck it up and don’t notify you. If that’s the api we’ve all agreed on, that’s fine. It was on you to check for the thing in the list first and then remove it. But it’s a bit weird if you think about it - now you need to traverse the list twice. Once to check for the existence (and fail out yourself) and once to remove.

christophilus 6y ago
I think a better approach is to return a bool or something like that, rather than crashing. But I agree that silently handling this case is the worst of the options.

adrianN 6y ago

I'm arguing that we should use asserts or similar defensive coding mechanisms to ensure that preconditions are met instead of merrily continuing to run our program with cascading error effects.

BoorishBears 6y ago
I've had this argument a ton with people used to new high level languages that make null safety "too easy" like: next?.doThing() Where doThing is never called if next is null. Languages that do this use "scary" operators to crash on null: next!!.doThing() And it's drilled into people's heads the latter is a Bad Thing (tm) - You really need to consider context in null handling. Imagine an app that alerts a nurse when the patients heartbeat is out of range. In an application where the UI context might have been closed out, it's common to see someUiContext?.showAlert() But what actually happens if the context is gone? It's better to crash and have part of your startup procedure be communicating that a crash occurred, and the doctor should check that something went wrong, than silently continuing. - The problem is when you tell people this, the kneejerk reaction is always "are you saying we intentionally add crashes"! Because safe null handling was specifically added to avoid the situation where that crashes... (For example, if the app was a news reader and the alert was "Article failed to load", you wouldn't want the app to crash just because the user left a certain page before the alert was shown) But I think the pendulum has swung too far at this point, people are so used to just sweeping nulls under the rug, and it's not great for finding issues
veets 6y ago
Yes - and that is the point the OP you responded to is making. This is not a generic library function. It used in a specific setting where preconditions exist and presumably are checked _prior_ to calling this code. Sure you could make the argument that we don't _know_ they are being checked, but it's a pointless discussion. Who cares? _If_ preconditions are met, this code is safe, if they aren't, it's not safe. Since we don't know one way or the other, there's no point in discussing further. The kernel developers know their stuff...
- adrianN 6y ago
  Imo every piece of code (for reasonable definitions of "piece") is supposed to check its own preconditions and not rely on the caller to check them.
  - veets 6y ago
    This is nice in theory but a bad idea in practice (as a blanket statement, I am all for checking preconditions in general). An easy example is binary search, which I think is a reasonable "piece" of code by your definition. One should never check its preconditions _inside_ the binary search function (that the list of elements being searched is partitioned by the search predicate). Checking the precondition is O(n) while the algorithm is O(lg n).
    - adrianN 6y ago
      You're right, but checking for null is not terribly expensive compared to iterating over a linked list.
      - HunterWare 6y ago
        The thing is that you aren’t really advocating for just that... Since this is pretty general support library code, the result of this philosophy is defensive checks in all support functions and that means checks every spinlock, mutex/semaphore, and god knows how many other places... everything really. We also would want these checks at multiple levels since we shouldn’t trust that other people checked things. This would have a significant effect on performance and many would prefer their system not to run however-many percent slower. All to avoid a possible issue that can’t be very large or systems would be kernel panic’ing all over the place. From a black box, zero-knowledge, perspective it’s maybe worth remembering that code built this way successfully runs mission critical systems all over the world every day. Thoughtful people would have switched to other (slower, more pedantic) systems if doing things differently was a real overall life improvement. People have had the choice, there are plenty of OS’s out there... some far more formal/pedantic. Linux wasn’t the safe choice back in the day, it was just better by several important metrics consistently over time. Perhaps these insanely experienced kernel devs know what they are doing.
andrepd 6y ago
Asserts have run-time cost. What you need is to ensure ensure those preconditions are met in the first place, statically, by formal methods for example.
- throwaway_pdp09 6y ago
  Which is ideal, but generally beyond reach for now. So assertions it is.

banachtarski 6y ago

What you are espousing is known as “defensive programming” which is very much a bad thing (tm).

adrianN 6y ago
I'm interested in hearing more about why defensive programming is bad. Do you have some links for me?
stuff4ben 6y ago
Clearly you've never worked with contract developers whose only purpose is to write a crap ton of code to get a "feature" out that crashes as soon as it encounters the real world. As they say, "doveryay, no proveryay" and it's served me well.
- hyperdimension 6y ago
  > As they say, "doveryay, no proveryay" I'm sorry if this might be common knowledge, but I've honestly never heard this and can't understand it. Is it saying that it's better to do something than to (formally) prove it, like the opposite of (I think) Knuth's famous quote?
  - DanBC 6y ago
    "trust, but verify". https://en.wikipedia.org/wiki/Trust,_but_verify https://en.wikipedia.org/wiki/Trust,_but_verify