2

Is it acceptable to have a post/prefix operator on the lhs of an assignment operator?

#include<ctype.h>
void to_upper_in_place(char* string)
{
    while(*string++ = toupper(*string));
}

If it is OK, is the previous considered readable C code, or should it be turned into something like the following?

#include<ctype.h>
void to_upper_in_place(char* string)
{
    while(*string) {
        *string = toupper(*string);
        string++;
    }
}

Or even:

while(*string = toupper(*string++));
samuelbrody1249
  • 4,379
  • 1
  • 15
  • 58
  • In many circumstances, it is absolutely readable, IMO. This is precisely the sort of thing that `++` and `--` are for. But you have to be careful when LHS and RHS use the same pointer, as they do here. – Steve Summit Feb 10 '21 at 23:40
  • @SteveSummit ok, thanks, is it considered better practice to put the post-increment on the lhs or rhs then? Or does it not matter at all? – samuelbrody1249 Feb 10 '21 at 23:40
  • 1
    Having such an operator on the LHS is okay in itself, but when the same variable appears elsewhere in the expression, you have UB in many cases (including yours). See https://stackoverflow.com/questions/949433/why-are-these-constructs-using-pre-and-post-increment-undefined-behavior?rq=1 – Nate Eldredge Feb 10 '21 at 23:40
  • Doesn't the fact that you have to *ask* if it is readable answer the question of if it is readable or not? – Scott Hunter Feb 10 '21 at 23:41
  • Actually, I spoke too soon. Comment edited. If you have the same pointer `string` on both sides, the answer changes quite a bit. – Steve Summit Feb 10 '21 at 23:41
  • *Is it acceptable to have a post/prefix operator on the LHS of an assignment operator?* yes, in general it is valid, however if you do that on the LHS then you can only use that variable on the LHS, using it anywhere else in the same assignment is undefined behavior. – Marco Bonelli Feb 10 '21 at 23:43
  • `for (int c; (c=*string) != 0; *string++ = toupper(c));` might be preferable. – mevets Feb 10 '21 at 23:52
  • 1
    To avoid UB when `*string < 0`, use `unsigned char* string`. – chux - Reinstate Monica Feb 11 '21 at 00:05
  • @chux-ReinstateMonica -- I see. What would be an example of when it would be `< 0` though? – samuelbrody1249 Feb 11 '21 at 00:08
  • 1
    @samuelbrody1249 `char` is either _signed_ or _unsigned_. When `char` is signed, it can have a value like -2, yet `toupper()` is only defined when `EOF` or for values in the `unsigned char` range. In general, C library treats characters as if they were `unsigned char`. So too should your code. – chux - Reinstate Monica Feb 11 '21 at 00:27
  • Like `void to_upper_in_place(char* string) { unsigned char *s = string; while(*s = toupper(*s)) { s++} }` – chux - Reinstate Monica Feb 11 '21 at 00:29

1 Answers1

7

Let's look at several different cases.

(1) (with two different pointers)

*dest++ = toupper(*src++);

This is perfectly ordinary C style.

(2)

*dest = toupper(*src);
src++;
dest++;

This is "longhand" style. You might write it if you were scared of using ++ in a dangerous way. (And a certain amount of caution is indeed justified, as we're about to see.)

(3) (back to the same pointer on lhs and rhs)

*p = toupper(*p);
p++;

This is fine.

(4)

*p++ = toupper(*p);

This is problematic, because p is both used and modified in the same expression, with the modification not depending on the value used. In many/most cases, this pattern renders the expression undefined, and that's a Bad Thing. It ends up being particularly hard to say wheteher this particular case is actually undefined, but for that reason we have to stay on the safe side, and say that (3) is preferable. (For more -- much more -- on this kind of undefined behavior, see this beleaguered old question.)

(5)

*p = toupper(*p++);

This is even worse. It's equally potentially undefined, but if it works at all, it probably increments p too soon.

(6) Yet another arrangement is to put the increment into the loop header, by using a for loop instead:

for(; *p; p++)
    *p = toupper(*p);

This is also fine.

Steve Summit
  • 45,437
  • 7
  • 70
  • 103