There is some problem with null character termination in this code

Question

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

int main(){

    char *str = (char*)malloc(4);

    strcpy(str,"aaab");

    char *pt;
    strcpy(pt,str);

    while(pt != NULL){
        printf("%c",*pt);
        pt++;
    }
}

I wanted this code to just print out the str string but instead it gave me the string and some bizzare output.The thing just gets better as the pattern keeps changing. I think the problem is with null character '\0' termination.

This is the code output.

Answer 1

There are several problems with the shown code.

The first one is that the string literal "aaab" contains 5 characters (including the terminating zero character '\0') instead of 4. So the call of strcpy that expects as the second argument a pointer to a string

char *str = (char*)malloc(4);

strcpy(str,"aaab");

invokes undefined behavior because the destination dynamically allocated memory is not large enough to store 5 characters.

You need to allocate one more byte to store the string

char *str = (char*)malloc(5);

In this code snippet

char *pt;
strcpy(pt,str);

there is used the uninitialized pointer pt and as a result again the call of strcpy invokes undefined behavior.

You could write for example

char s[5];
char *pt = s;

strcpy(pt,str);

And this while loop has undefined behavior even if the pointer pt would be initialized

while(pt != NULL){
    printf("%c",*pt);
    pt++;
}

Instead you should compare not the pointer itself with NULL but the character pointed to by the pointer with the terminating zero character '\0' like

while(*pt != '\0'){
    printf("%c",*pt);
    pt++;
}

Of course the pointer pt shall point to a string.

Pay attention that you should free the allocated memory when it is not required any more

free( str );

Answer 2

strcpy(str,"aaab")

1. You're trying to write 5 bytes to a location you allocated 4 bytes for. Recall that the C strings are terminated with a null-byte, and the valid indexing range is from 0 to n - 1.

char *ptr;

2. This only allocated memory for the pointer. The contents of the pointer are indeterminate, i.e. they may be pointing to anything and accessing the memory pointed to by the pointer invokes undefined behaviour. You need to allocate memory for the pointed-to data before trying to copy something to it.

3. Do not cast the result of malloc(). malloc() and family returns a generic void * that is implicitly converted to any other pointer type. The cast is redundant and just clutters the code.

4. Check the return value of malloc(). It returns a NULL pointer constant to indicate failure.

5. Compare p's pointee to the null-byte, not the NULL pointer constant.

// while (p != NULL) {
} 

while (*p != `\0`) {
}

Or simply;

while (*p) {
}

Otherwise, your code would invoke undefined behaviour.

Answer 3

When malloc(4); is used like this, it provides only 4 bytes, but for \0 character at least one more byte is required. So I suggest to use

char *str = (char*)malloc(5);

But more solid solution is to count length of source string, add 1 to this value and allocate memory for destination string. E.g.:

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

int main(){
    char *s1 = "aaab";
    char *s2 = (char*)malloc(strlen(s1) + 1);

    strcpy(s2, s1);

    printf(">%s<", s2);
}