The function has quite questionable implementation. It fails miserably for strings with length < i.
The function has quite questionable implementation. It fails miserably for strings with length < i.
Still, this function is called only once, and in that call, its i argument is always <= length, so it's perfectly fine (it's only UB if you actually pass it a bad argument).
This very mindset is a source of bugs and vulnerabilities. The author has high marks from me on safety and "make it hard to use wrong" and it's quite surprising to see such code.
The insanity would be assuming that every function is valid for the Cartesian product of all possible of its arguments.
What he probably needs is an assert
> The insanity would be assuming that every function is valid for the Cartesian product of all possible of its arguments.
Would it? That reminds me of a recent post on HN about proving the long (binary) division algorithm with Hoare's logic. It uses the "d > 0" precondition and proves that, indeed, the algorithm arrives at the required postcondition. However, the algorithm still terminates and produces something even when d == 0. What does it computes in this case? Is it useful? Should such questions even be considered?
Yes, a better understanding of the problem gives you a better understanding of the preconditions. Always ask if you have that right and weaken accordingly.
In general you can write better code when you can make assumptions.
Code to handle every possibility is filled with error prone branching, that reduplicates effort at every function.
This isn't some 100k line long program where this function is used all over the place and code churns constantly so checking invariants in the function definition makes sense.
It is called in one (1) place in a small program.
It could use `str*` functions without any issues then. Nul-terminated strings are perfectly safe with assumptions to follow.
Anecdote: I fixed 3 reported segfaults and another 2 after fuzz-testing in a small 500 line lib. Original author had the same cowboy mindset about keeping all stuff in his head. It's always last words before getting into CVE database.
Nobody is saying that. Name, document, and assert.
Good code consists of easy to use abstractions. In general Chris's blog is dedicated to poking bad abstractions and giving good examples. `substring` is objectively bad.
You (and other commentators) literally arguing that keeping staff in the head or making some documentation or notes or forcing yourself or others check low level implementation details are better than making one trivial fix and forget about it. I find it really amusing.
While it is "obvious" that it is correct, it is, in fact, neither obvious nor correct if A. you ever subtract from then len field, and B. you use "len != 0" as a check for "is string non-empty". This program does both things quite a lot and it requires a conscious check to re-assure yourself that no, in this particular program, the patterns of acitons A (subtractions from the len field) never break the property B (non-zero len means that the string is not empty). But any minor modification may accidentally break this harmony if you are not aware that it is required to hold.
Good code consists of solving a problem simply and efficiently. You can have a great program which is concrete. Abstraction is a tool for managing complexity, not a goal.
I would really encourage writing a large program C or assembly, as a practice of how to deal with programming problems without immediately wrapping things in an interface. A lot of things that feel like producitivity end up being wastes of time.