Error in assigning values to memory location with for loop

Question

I have just entered the world of C programming, and I'm currently learning how to use malloc & free.

I've written a short excercise code to printf the string entered with scanf, and I am having trouble compiling it.

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

int main(void)
{
    char *keyword;
    printf("keyword: ");
    scanf("%s", keyword);
    if (keyword == NULL)
        return 1;

    char *t = malloc(strlen(keyword) + 1);
    if (t == NULL)
        return 1;

    for (int i = 0, n = strlen(keyword) + 1; i < n; i++)
        t[i] = keyword[i];

    printf("t: %s\n", t);

    free(t);
    return 0;
}

To debug it, I wrote the same code but without the for loop(code below), and then the code worked well. This led me to conclude that the issue might be with the for loop which is used to assign characters to corresponding memory locations, but I am still unable to find a solution.

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

int main(void)
{
    char *keyword;
    printf("keyword: ");
    scanf("%s", keyword);
    if (keyword == NULL)
        return 1;

    char *t = malloc(strlen(keyword) + 1);
    if (t == NULL)
        return 1;

    t[0] = keyword[0];
    t[1] = keyword[1];
    t[2] = keyword[2];
    t[3] = keyword[3];
    t[4] = keyword[4];
    t[5] = keyword[5];
    printf("t: ");
    printf("%c", t[0]);
    printf("%c", t[1]);
    printf("%c", t[2]);
    printf("%c", t[3]);
    printf("%c", t[4]);
    printf("%c", t[5]);
    printf("\n");

    free(t);
    return 0;
}

I am pretty sure this is a simple error, but I cannot figure out the reason of this error with debugger or online. I would really appreciate it if someone could help me solve this problem.

Answer 1

The following lines:

char *keyword;
printf("keyword: ");
scanf("%s", keyword);
if (keyword == NULL)
     return 1;

Have several problems:

1. keyword is not initialized. It must be initialized to point to some buffer/memory in order to scanf into it in scanf("%s", keyword);.
   Accessing it uninitialized causes undefined behavior (UB).
   In order to initialize it you should determine the maximum length you allow for it, and then initialize it by:
   (1) Using malloc
   (2) Using stack allocation (if the max size is relatively small)
   (3) Setting it to point to some existing buffer
   Don't forget to add 1 to the max length when you allocate, for zero termination.

2. After determining the maximum length of keyword you should pass this value to scanf to make sure the buffer is not overrun, e.g.:

   ... = scanf("%33s", keyword);  // here the maximum length is 33
   

3. You should always check the return value of scanf. If it succeeds it returns:

   Number of receiving arguments successfully assigned

   (here it should be 1).

4. (keyword == NULL) is unlikely to be true (unless by chance), regardless of whether scanf succeeded or not. And if you'll initialize it properly as explained above, it will never be NULL.