Comment by Progge
1 month ago
Long time ago I wrote C. Could anyone fill me in why the first code snippet is arg parsing the way it is?
int main(int argc, char* argv[]) {
if (argc > 1) {
char\* r_loc = strchr(argv[1], 'r');
if (r_loc != NULL) {
ptrdiff_t r_from_start = (r_loc - argv[1]);
if (r_from_start == 1 && argv[1][0] == '-' && strlen(r_loc) == 1) {
in_reverse = 1;
}
}
}
...
}
Why not
if (argc > 1 && strcmp(argv[1], "-r") == 0) {
in_reverse = 1;
}
for example?
It doesn't even make sense to use strchr for determining the position of 'r', when the code checks that the position of '-' is at index 0.
Your solution is perfectly fine. Even if you don't have access to strchr for some reason, the original snippet is really convoluted.
You could just write (strlen(argv[1]) > 1 && argv[1][0] == '-' && argv[1][0] == 'r') if you really want to.
It could make some sense to use strchr, because in idiomatic UNIX tools, single character command line options can be clustered. But that also means that subsequent code should not be tested for a specific position.
And if you ever find yourself actually doing command line parsing, use getopt(). It handles all the corner cases reliably, and consistent with other tools.
Of course, `&&` in C is short-circuiting so it's safe without the `strlen()` too, as long as the argument is there i.e. not NULL.
Also, the use of a convoluted `if` to conditionally assign a literal boolean is a code smell (to me), I would drop the `if` and just use:
if a more forward-thinking/strict check is not needed.
Your code actually has 2 bugs. The first I assume is just a typo and you meant to use [1][1] == ‘r’. The second one is that you would accept “-rblah” as well.
Not to mention the potential signed integer overflow in (*right - *left) and (*left - *right), which is undefined behavior. And even if you rely on common two's complement wraparound, the result may be wrong; for example, (INT_MAX-(-1)) should mathematically yield a positive value, but the function will produce INT_MIN, which is negative.
And then we have this "modern" way of spelling pointers, "const int* right" (note the space). In C, declaration syntax mirrors use, so it should be "const int *right", because "*right" is a "const int".
I feel too old for this shit. :(
I wonder if there is a way to actually do it with only arithmetic, without comparisons?
Using only arithmetic operators would require explicit checks for overflow and this would be a very inefficient implementation, because the comparison operators handle overflow implicitly without any overhead (because the comparison operators do not use the sign of the subtraction result to decide their truth value, but they compute the true sign of the result, by the modulo 2 sum, a.k.a. XOR, of the result sign with the integer overflow flag; this is done in hardware, with no overhead).
The feature that integer comparison must be correct regardless of overflow has been a requirement in CPU design since the earliest times. Very few CPUs, the most notable examples being Intel 8080 and RISC-V, lacked this feature. The competitors and successors of Intel 8080, i.e. Motorola MC6800 and Zilog Z80 have added it, making them much more similar to the previously existing CPUs than Intel 8080 was. The fact that even microprocessors implemented with a few thousand transistors around 1975 had this feature emphasizes how weird is that RISC-V lacks such a feature in 2025, after 50 years and being implemented with a number of transistors many orders of magnitude greater.
7 replies →
I prefer (with "greater" being ±1, defaulting to +1):
You are right, the implementation of the "compare" function should have used only comparison operators, not subtraction, because unlike subtraction the comparison operations are not affected by overflow (the hardware implementation of integer comparison handles overflow automatically).
When there is no overflow, the sign of the subtraction result provides the same information as a comparison operator, but this is no longer true when overflow happens.
I suspect it was adopted from a bigger snippet that had support for parsing things like "-abc" as "-a -b -c", etc.