Elijah Newren <newren@xxxxxxxxx> writes:
> It's identical for runtime correctness, yes. But the primary point
> isn't what happens at runtime, but what happens when future code
> readers come along. If I only keep the "if (dst->is_rename)" line
> that comes after without the assert, then someone in the future will
> come along (maybe even myself) and think "the original author wasn't
> being careful here; they should change this to a check on (dst &&
> dst->is_rename)" (because honestly, it is the kind of mistake I'm
> prone to make). However, if they were to do so, and some bug gets
> introduced so that locate_rename_dst() returns a NULL for a pair of
> interest, then they've masked a bug in the algorithm and made it fail
> in harder-to-detect-and-track-down ways. I wanted it to be clear that
> dst == NULL is unacceptable. I guess I could have marked it with
> BUG(), but between an assertion and a NULL-pointer indirection, I
> figured that code aborting under that condition was pretty well
> covered. :-)
assert() often gets turned into no-op, so it is more like leaving a
comment in the code. A comment or BUG("text") can describe not just
the fact that reaching this point with dst==NULL is a bug in the
caller but also why it is a bug (e.g. how NULL-dst would screw up
the computation downstream from this point), but an assert() cannot.
