3

During clean compile warning, I met following code:

char *strcpy(char *dest, char *src)
{
    unsigned int i;

    while( dest[i] = src[i] )//assignment in condition
        i++;

    return dest;
}

the base function of the code should be OK, but compiler warn that the assignment in condition, does this part of code have any potential risks? if this kind of warning need be cleaned?

How Chen
  • 1,340
  • 2
  • 17
  • 37

2 Answers2

4

All kinds of warnings need to be cleaned.

This warning was introduced because = and == is often confused by programmers (hello, Pascal!), and your intent may be explicitly stated by adding parentheses around assignment expression:

if ((x = y)) // no warning
user3125367
  • 2,920
  • 1
  • 17
  • 17
3

No, you don't need to fix this.

The reason for the warning is that sometimes people mistakenly type = when they mean ==, e.g.

while (response = 'n')

An assignment in a conditional is more likely to be a mistake like this than an assignment whose value you want to test, so the compiler warns about it. You can silence the warning by wrapping the assignment in a test:

while ((dest[i] = src[i]) != 0)

On the other hand, I recommend that you always put the body of a while or if inside {}, even if it's just one statement. See Why is it considered a bad practice to omit curly braces?

You also need to initialize i:

unsigned int i = 0;
Community
  • 1
  • 1
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • I would second that advice. Even though it is legal to do an assignment inside a conditional, it makes the code harder to read. The compiler optimizer is likely to generate the same code if you are more explicit in your intent. Many years (decades) ago, devs could help optimize code by using certain short hands. That is no longer true. – KC-NH Oct 17 '14 at 02:20
  • 1
    The worst of all bad practices is "no need to fix this". You'll never notice an actual issue in tons of not-so-important warnings left from previous "successful" builds. – user3125367 Oct 17 '14 at 02:34
  • While that may be true in general, this particular one has some notable false positives. Consider the common `while (row = fetch_from_db())` pattern. – Barmar Oct 17 '14 at 02:47
  • Obviously they do not use/read/fix warnings where it is common. Some popular projects even follow the "build as with -Werror under all supported compilers" idiom. – user3125367 Oct 17 '14 at 02:58