Remix.run Logo
Progge 11 hours 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?

tapete2 11 hours ago | parent | next [-]

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.

microtherion 11 hours ago | parent | next [-]

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.

unwind 8 hours ago | parent | prev | next [-]

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:

    in_reverse = argc > 0 && argv[1][0] == '-' && argv[1][1] == 'r';
if a more forward-thinking/strict check is not needed.
eska 5 hours ago | parent | prev [-]

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.

CerryuDu 2 hours ago | parent | prev | next [-]

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. :(

Joker_vD 8 hours ago | parent | prev | next [-]

I suspect it was adopted from a bigger snippet that had support for parsing things like "-abc" as "-a -b -c", etc.

4 hours ago | parent | prev [-]
[deleted]