2

Just trying to scan N strings to an array and dynamically allocate them

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

#define N 3
#define MAX_STR_LEN 20

void main(void) {
    char* str_arr[N] = { '\0' };
    char temp_str[MAX_STR_LEN] = { '\0' };

    for (size_t i = 0; i < N; i++)
    {
        //scan string
        fgets(temp_str, MAX_STR_LEN, stdin);
        strtok(temp_str, "\n");

        // next line of code triggers breakpoint at second iteration
        *(str_arr + i) = (char*)calloc((strlen(temp_str) + 1), sizeof(char));
        if (!(str_arr + i))
        {
            printf("Could not allocate memory\n");
            return 1;
        }
        //copy temporary string to dedicated string
        strcpy_s(*(str_arr + i), sizeof(temp_str), temp_str);
    }

    printf("\n");
    system("PAUSE");
}
James Blue
  • 90
  • 6

1 Answers1

3

A few observations:

  • You didn't allocate sizeof(temp_str). You allocated strlen(temp_str)+1. So the first strcpy_s is going to cause a buffer overrun, which is why things go bad on the second iteration through the loop.
  • The statement if (!(str_arr +i)) doesn't check the pointer that you just allocated
  • *(str_arr + i) is the same as str_arr[i] and the latter is easier for someone reading the code
  • In the line char* str_arr[N] = { '\0' };, you declare an array of pointers, but you initialize with a character constant (\0) which is an int. The initializer should be a NULL pointer.
  • This answer shows a better way to remove the newline from the buffer. The problem with strtok is that it won't remove the newline if/when the user enters a blank line.

With that in mind, here's my recommended fix:

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

#define N 3
#define MAX_STR_LEN 20

int main(void) {
    char* str_arr[N] = { NULL };
    char temp_str[MAX_STR_LEN] = { '\0' };

    for (size_t i = 0; i < N; i++)
    {
        //scan string
        if (fgets(temp_str, MAX_STR_LEN, stdin) == NULL)
        { 
            printf("Not enough lines of input\n");
            return 1;
        }
        temp_str[strcspn(temp_str, "\n")] = '\0';

        size_t length = strlen(temp_str) + 1;
        str_arr[i] = (char *)calloc(length, 1);
        if (!str_arr[i])
        {
            printf("Could not allocate memory\n");
            return 1;
        }
        //copy temporary string to dedicated string
        strcpy_s(str_arr[i], length, temp_str);
    }

    printf("\n");
    system("PAUSE");
}
user3386109
  • 34,287
  • 7
  • 49
  • 68
  • 1
    OP used “char* str_arr[N] = { '\0' };” and you change it to “char* str_arr[N] = { NULL };”. Maybe you want to explain to the OP, why you did that. – Michi Mar 23 '18 at 00:36
  • 1
    Although ignoring the return of `strtok` works for this specific case, it might be better to utilize it and explain [to OP] why that's better for the general case – Craig Estey Mar 23 '18 at 00:39
  • Re bullet 4: Integer 0 is freely convertible to the NULL pointer and back. – user58697 Mar 23 '18 at 00:52
  • 1
    @user58697 And if you turn the warnings up, you get *"warning: expression which evaluates to zero treated as a null pointer constant of type 'char *' [-Wnon-literal-null-conversion]"*. So yes, you can do that, but you shouldn't. – user3386109 Mar 23 '18 at 00:59