Rafay Khan Rafay Khan - 4 months ago 11
C++ Question

Why is "while (*sea++ = *river++);" not working as it should?

My college professor recently gave us a task of implementing our own smart pointer classes. In his boilerplate code for copying strings I found this piece beautiful syntactic sugar:

while (*sea++ = *river++);// C Sting copy


I looked further into this code and found that it is the exact code as found in strcpy.c and further explanation of how it works, in the following stackoverflow question:
How does “while(*s++ = *t++)” copy a string?

When I tried to use this syntactic sugar in my following code it gave garbage as a result and deleted the string stored in "river":

#include<iostream>
#include<cstring>

using namespace std;
void main()
{
const char *river = "water";// a 5 character string + NULL terminator

char *sea = new char[6];

while (*sea++ = *river++);

cout << "Sea contains: " << sea << endl;
cout << "River contains: " << river << endl;
}


RESULT:

Result of the above code

I know that I can simply achieve the desired result with the following code instead:

int i = 0;
while (i<6)
{
sea[i] = river[i];
i++;
}


But this is not the answer I want. I would like to know is there something wrong with the implementation of my while loop or the instantiation of my char pointers?

Answer

The "error" is technically an undefined behavior due to pointer gone out of bound respect to to the memory they refer.

The interesting part is WHY that happens.

And it is due by the confusion between the pointers and what they point to.

sea and river are not strings. The strings are anonymous variable residing somewhere in memory, with the two pointer indicating their start.

You should never touch them, otherwise you will be no more able to access those strings further.

If you need to move pointers, use other ones.

A more proper example should be this one

 using namespace std;
 int main() //< note `void main` is a C++ dialcet
 {
     // not the use of `const` after the `*`: 
     // you cannot modify this pointers.
     const char * const river = "water";  // a 5 character string +  NULL terminator
     char * const sea = new char[6];

     {
          // in this inner scope, other non-const pointer are
          // initialized to point to the same memory
          const char* r = river;
          char* s = sea;
          while (*s++ = *r++); // the loops moves the mutable pointers
          // note how `river++` or `sea++` is an error, being them `*const`
     }

     // there are no more `r` and `s` here, but `sea` and `river` are still the same.
     cout << "Sea contains: " << sea << endl;
     cout << "River contains: " << river << endl;

     //now we have an array allocated with new to return to the system
     delete[] sea; //< the importance to not move the `sea` pointer
}

Note how delete deletes the array, not the pointer.

To go more in advance, two things can be done.

The first is make the inner scope a proper function:

 using namespace std;

 void copy(const char* r, char* s)
 {
     // in this function, other non-const pointer (parameters) are
     // initialized to point to the same memory upon call
     while (*s++ = *r++); // the loops moves the mutable pointers
     // note how `river++` or `sea++` is an error, being them not visible.
 }

 int main() //< note `void main` is a C++ dialect
 {
     const char * const river = "water";  // a 5 character string +  NULL terminator
     char * const sea = new char[6];

     copy(river, sea);         

     cout << "Sea contains: " << sea << endl;
     cout << "River contains: " << river << endl;

     //now we have an array allocated with new to return to the system
     delete[] sea; //< the importance to not move the `sea` pointer
}

and the second is get rid of the new/delete pair in a same context, using -for example- an std::unique_ptr<char[]>

But this is taking too far!