Why does this code to modify a string not work?

Question

With c-style strings, how do you assign a char to a memory address that a character pointer points to? For example, in the example below, I want to change num to "123456", so I tried to set p to the digit where '0' is located and I try to overwrite it with '4'. Thanks.

#include <stdio.h>
#include <stdlib.h>

int main()
{
    char* num = (char*)malloc(100);
    char* p = num;

    num = "123056";

    p = p+3;    //set pointer to where '4' should be
    p = '4';

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

    return 0;
}

Solution 1

That code won't work, simply because the line:

num = "123056";

changes num to point away from the allocated memory (and p remains pointing to the that memory so they're no longer the same location) to what is most likely read-only memory. You are not permitted to change the memory belonging to string literals, it's undefined behaviour.

You need the following:

#include <stdio.h>
#include <stdlib.h>
int main (void) {
    char *num = malloc (100); // do not cast malloc return value.
    char *p = num;

    strcpy (num, "123056");   // populate existing block with string.

    p = p + 3;                // set pointer to where '0' is.
    *p = '4';                 // and change it to '4'.

    printf ("%s\n", num );    // output it.

    return 0;
}

Solution 2

First of all, when you do:

num = "123056";

You are not copying the string "123056" to the area of heap allocated by malloc(). In C, assigning a char * pointer a string literal value is equivalent to setting it as a constant - i.e. identical to:

char str[] = "123056";

So, what you've just accomplished there is you've abandoned your sole reference to the 100-byte heap area allocated by malloc(), which is why your subsequent code doesn't print the correct value; 'p' still points to the area of heap allocated by malloc() (since num pointed to it at the time of assignment), but num no longer does.

I assume that you actually intended to do was to copy the string "123056" into that heap area. Here's how to do that:

strcpy(num, "123056");

Although, this is better practice for a variety of reasons:

strncpy(num, "123056", 100 - 1);  /* leave room for \0 (null) terminator */

If you had just done:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int     main(void) {
        char    *num = malloc(100);
        char    *p = num;

        strncpy(num, "123056", 100 - 1);

        p = p + 3;
        *p = '4';

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

       return 0;
} 

You would have gotten the correct result:

123456

You can contract this operation:

p = p + 3;
*p = '4';

... and avoid iterating the pointer, by deferencing as follows:

*(p + 3) = '4';

A few other notes:

• Although common stylistic practice, casting the return value of malloc() to (char *) is unnecessary. Conversion and alignment of the void * type is guaranteed by the C language.

• ALWAYS check the return value of malloc(). It will be NULL if the heap allocation failed (i.e. you're out of memory), and at that point your program should exit.

• Depending on the implementation, the area of memory allocated by malloc() may contain stale garbage in certain situations. It is always a good idea to zero it out after allocation:

  memset(num, 0, 100);
  

• Never forget to free() your heap! In this case, the program will exit and the OS will clean up your garbage, but if you don't get into the habit, you will have memory leaks in no time.

So, here's the "best practice" version:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int     main(void) {
        char    *num, *p;

        /*
         * Don't take 1-byte chars for granted - good habit to get into.
         */

        num = malloc(sizeof(char) * 100);

        if(num == NULL)
                exit(1);

        memset(num, 0, sizeof(char) * 100);

        p = num;

        strncpy(num, "123056", 100 - 1);

        *(p + 3) = '4';

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

        free(num);

        return 0;
}

Solution 3

In addition to the *p issue others have pointed out, you also have memory usage issues. You have one buffer of 100 bytes, with unknown contents. You have another buffer of 7 bytes, containing the string "123056" and a null terminator. You're doing this:

1. num is set to point to the 100 byte buffer
2. p is set to point to num; ie, it points to the 100 byte buffer
3. you reset num to point to the 7 byte buffer; p is still pointing to the 100 byte buffer
4. You use p to modify the 100 byte buffer
5. then you use num to print out the 7 byte buffer

So you're not printing the same buffer that you are modifying.

Comments

[Mike Anderson]

Dereferencing it didn't work. Is the string 'num' immutable or something? I'm compiling with cc on linux.

[Mike Anderson]

Thank you Bruce, your explanation was helpful but Pax's strcpy really made it clear.

[Igor Krivokon]

Also, need to free the memory.

[paxdiablo]

That's good form, @Igor, but not necessary given the program is exiting straight away. And the idea of a snippet is to convey relevant information, not all information, otherwise, we need to check that the malloc() worked as well.

[sharptooth]

Yes, the string literal is likely allocated in a memory segent that is marked read-only and any attemp to write into it causes a runtime error.

[paxdiablo]

One nitpick, the address '4' is not illegal. Implementations can quite easily use memory at 0x00000034 (it's even aligned correctly for a 32-bit word).

[Alex Martelli]

@Igor, good spotting, editing to fix, tx -- @sharptooth, that's very depended on yr compiler &c, a strdup might help.

[Sefler]

Well, I said illegal for this application. It is a memory address unknown.

[Mike Anderson]

Thanks for the great memory management tips. These tips are good for people who come from automatic garbage collection languages!

[Olaf Seibert]

sizeof(char) is defined to be 1. see 6.5.3.4 The sizeof and _Alignof operators: 4 When sizeof is applied to an operand that has type char, unsigned char, or signed char, (or a qualified version thereof) the result is 1.