5

I have a code block:

int main ()
{
    char *p1 = "Hello";
    char *p2;
    p2 = (char*)malloc (20);
    memset (p2, 0, 20);
    while (*p2++ = *p1++);
    printf ("%s\n", p2);
}

But I can not explain the working of the line while (*p2++ = *p1++); Could you let me know the order of operation in this formula?

sashoalm
  • 75,001
  • 122
  • 434
  • 781
mommomonthewind
  • 4,390
  • 11
  • 46
  • 74
  • 1
    [How does “while(*s++ = *t++)” work?](http://stackoverflow.com/q/810129/4279) – jfs May 16 '13 at 13:54

3 Answers3

18

It's classic C code trying to look extremely clever by putting everything in one line.

while (*p2++ = *p1++); is equivalent to

strcpy(p2, p1);
p1 += strlen(p1) + 1;
p2 += strlen(p2) + 1;

In other words, it copies a null-terminated string, with p1 ending up pointing to the end of the source string and p2 pointing to the end of the destination string.

user541686
  • 205,094
  • 128
  • 528
  • 886
  • 6
    +1 for the comment about trying to look clever. – Theodoros Chatzigiannakis May 16 '13 at 07:18
  • 5
    It's only *functionally* equivalent. The equivalent code posted has additional overhead. strlen() calls iterate over the strings *again*. – P.P May 16 '13 at 07:29
  • 18
    Yeah, this isn't anything crazy. Heck, it appears in KR, and I don't think you would accuse them of trying to be "overly clever". Your version is unnecessarily expensive. – Ed S. May 16 '13 at 07:31
  • Then simply replace the strlen calls with `p1 += sizeof("Hello"); p2 += sizeof("Hello");`, then it would be equivalent. Though, there is no obvious reason why the program would need the p1 and p2 pointers to point at that memory location. I'd rather prefer to have p2 to still point at the first dynamically allocated cell, so that there would be no memory leak. – Lundin May 16 '13 at 07:33
  • 1
    @Lundin: Then create a temp and iterate over that. Technically there is a leak (and the `printf` is wrong), but the program immediately exits. – Ed S. May 16 '13 at 07:34
  • @EdS. Formally, the OS isn't obliged to clean up the mess you left behind. Though of course in practice, every OS does so. – Lundin May 16 '13 at 07:35
  • 6
    nothing overly clever, just normal C code IMO. in fact strcpy() is often implemented like that. – AndersK May 16 '13 at 07:52
  • 1
    @KingsIndian, @Ed S: It's possible that Mehrdad's code is faster; it depends on the implementation. Smart implementations of strcpy() and strlen() access the string 4 or 8 bytes at a time, which can be faster than the byte-by-byte access in the original. And of course, if one piece of code happens to trigger compiler optimizations then that can cause a major difference in speed (e.g. if the compiler figures out the length of `p1` and unrolls the loop or uses memcpy() instead of strcpy()). – user9876 May 16 '13 at 09:33
  • @user9876 I am not sure how any implementation can access 4-8 bytes in strlen()/strcpy(). *Any* byte can be `0` and terminate the string and thus can't access arbitrary number of bytes without invoking undefined behaviour. Do you have any examples of such "smart" implementations? I don't understand how you say Mehrad's code could be "faster". Because the original code doesn't require any pointer movements as it's done as part of the copying process. Whereas Mehrad's code has *extra* loops in the form of strlen(). – P.P May 16 '13 at 10:25
  • @KingsIndian: I didn't say it's the same, I said *equivalent*. The two pieces of code are certainly equivalent despite their performance characteristics. – user541686 May 16 '13 at 12:34
  • 1
    @EdS.: Coding practices have evolved a lot since K&R, in case you hadn't noticed. They might not have been trying to look clever back then (or they might have been -- I don't know), but this certainly *does* look like it's trying hard to be clever nowadays. There's a reason Python stopped allowing assignments inside conditions. – user541686 May 16 '13 at 12:39
  • @KingsIndian: For your information, not every compiler is as dumb as you think. Some compilers [can optimize away `strlen`](http://stackoverflow.com/a/5770845/541686). Your comment about "any byte being zero" shows a lack of understanding -- the **implementation** certainly can make assumptions that the user code might not be able to (e.g. accessing bytes past the end); what's undefined for the user isn't necessarily undefined for the implementation. – user541686 May 16 '13 at 12:47
  • @Mehrdad I know compilers *may* optimize the call to strlen. Not every compiler is *obliged* to optimize it. My point is that call itself is unnecessary. What's the point in discussing whether call is efficient or compiler optimizes it? – P.P May 16 '13 at 14:21
  • Re `implementation certainly can make assumptions that the user code might not be able to (e.g. accessing bytes past the end`. If I do `char *p=malloc(10); strcpy(p, "ABC");` then only the first 4 bytes can be accessed and accessing rest of the uninitialized locations *can* invoke UB. The implementation at best make assumption that user code can't access beyond 10 bytes (as you claim). But even accessing within the allocated 10 bytes *can* be UB. Perhaps, you can enlighten me. – P.P May 16 '13 at 14:32
  • @KingsIndian: On a 64-bit system, loading 8 bytes into a 64-bit register can take the same number of cycles as loading 1 byte into a 8-bit register. That's 8x more bytes read per cycle, which is faster. If the compiler knows that it's an architecture where pages are 4096 bytes and 4k-aligned, then reading 8 bytes at a time is totally safe so long as the reads are aligned to an 8-byte boundary (you may need to read the first few characters one at a time to get this). GCC/glibc `strlen` actually does this optimization. – user9876 May 16 '13 at 16:47
  • (Note that the compiler & standard library can do things that a portable C program can't, simply because the compiler & standard library don't need to be as portable). – user9876 May 16 '13 at 16:49
  • 1
    @Mehrdad: It's really not. If you don't immediately know that that loop is performing a string copy then you simply don't know C. Now that's fine, everyone starts somewhere, but who writes their code such that the most inexperienced programmers will never have to think about what the code is actually doing? They're not the target audience, and C is not Python. – Ed S. May 16 '13 at 17:01
  • There's a difference between the bytes you say (read by CPU) and the byte I said (which is actual reading the bytes to verify if it's 0-byte). There's no disagreement in light of your recent comments. Perhaps, you were not very explicit or may be, I didn't think well enough to understand what you really mean. Nevertheless, my comments are valid that calls to strlen() itself are unnecessary and it doesn't matter how efficiently it can be done. i.e. The original code is NOT *extremely clever* and is quite a usual one to copy strings in C. – P.P May 16 '13 at 17:08
9

This is a string copy, but you are losing the original pointer value. You should save the original pointer value.

int main ()
{
    char *p1 = "Hello";
    char *p2 = malloc(20);
    char *p3 = p2;
    memset (p2, 0, 20);
    while (*p2++ = *p1++);
    printf ("%s\n", p3);
}

The actual semantic explanation of the while loop would be something like:

for (;;) {
    char *q2 = p2;              // original p2 in q2
    char *q1 = p1;              // original p1 in q1
    char c = *q1;               // original *p1 in c
    p2 += 1;                    // complete post increment of p2
    p1 += 1;                    // complete post increment of p1
    *q2 = c;                    // copy character *q1 into *q2
    if (c) continue;            // continue if c is not 0
    break;                      // otherwise loop ends
}

The order that q1 and q2 are saved, and the order that p2 and p1 are incremented may be interchanged. The save of *q1 to c can occur any time after q1 is saved. The assignment of c to *q2 can occur any time after c is saved. On the back of my envelop, this works out to at least 40 different interpretations.

jxh
  • 69,070
  • 8
  • 110
  • 193
  • The original code also created a memory leak. – Lundin May 16 '13 at 07:27
  • @Lundin: It didn't `free()` the memory that was allocated, but the program is exiting. It's only a leak to memory debugging tools. But, yes, generally, every `malloc()` (or similar) should be accompanied by a `free()`. The original code makes it hard to recover the original pointer. – jxh May 16 '13 at 07:31
  • 1
    It's not undefined behaviour, because of the `memset`. The `printf` in the original program is guaranteed to print just `\n`. – user9876 May 16 '13 at 09:18
  • @user9876: Thanks, answer is updated. – jxh May 16 '13 at 15:14
1

The while loop is evaluating the expression: *p2++ = *p1++. The while loop expression:
*p2 = *p1 is evaluated using the result of *p1. However, this value is still assigned to *p2 even if the expression evaluates as false or (0). Rewriting this:

char c;

do
{
    c = *p1; /* read the src byte */
    *p2 = c; /* write to dst byte */

    p2++, p1++; /* increment src, dst pointers */
}
while (c != 0);

You will notice that a read / write will occur at least once. That's OK, as long as the C string p1 is nul-terminated, and p2 has enough storage for the C string. That is, malloc should allocate at least strlen(p1) + 1 bytes. In this code provided, this is true.

As others have pointed out, the final iteration will leave p1 at an address one-past-the-end, which is still a valid pointer, but has undefined results when dereferenced. The address of p2 is both a valid pointer, and a valid dereference, since you're allocating 20 bytes. However, p2 does not point to the C string copy any longer. What you want is an equivalent to:

char *p1 = "Hello";
char *p2, *tmp;

p2 = (char*)malloc (20);
memset (p2, 0, 20);

tmp = p2;
while (*tmp++ = *p1++);

printf ("%s\n", p2);

Most operating systems will release the memory at p2 on exit from main, but it is good practice to unwind the resources with a corresponding call to:

free(p2);

at the end. While on the subject of good practice, you should also check the return value of malloc to ensure that the allocation succeeded.

Brett Hale
  • 21,653
  • 2
  • 61
  • 90
  • 1
    `p2` *does* point to a nul-terminated C string. That's because the `p2` buffer was allocated larger than `p1`, and was zeroed with `memset` – user9876 May 16 '13 at 09:21