I want to put some strings at dynamic 2D array with pointers(C programming)

Question

Hi I want to get some strings from user then put those in 2D array and at the end just print every character position in array for test.(Im writing at visual studio 2017) but I'm getting output like this for example:

    marray[1][2]==   
    marray[1][3]==
    marray[1][3]==

and I'm getting this for all cells. and here is my code so far:

#include <stdio.h>  
#include <stdlib.h>
void inputnames(char**, int, int);
void printitems(char**, int, int);
int main(void)
{
    int m;
    const int n = 30;  //students name limit
    printf("Enter Number of students:");
    scanf_s("%d ", &m);   //getting row size from user

    //---------------------------------------------
    char** p;
    p = (char**)malloc(sizeof(char)*m);
    for (int i = 0; i < m; i++)
    {                               //this part for allocating 2d array 
        p[i] = (char*)malloc(sizeof(char)*n);
    }
    //--------------------------------------------
    inputnames(p, m, n);        //filling 2D array
    printitems(p, m, n);        //print each character with position for test
    getchar();
}
void inputnames(char** marray, int mm, int nn)
{
    int i = 0, j = 0;
    for (i = 0; i<mm; i++)
    {
        while (marray[i][j] != '\n' && j<nn)
        {
            scanf_s("%c", &marray[i][j]);
        }
    }//end of for i
}
void printitems(char** marray, int mm, int nn) //this function is for test 
{
    int i = 0, j = 0;
    for (i = 0; i<mm; i++)
        {
            for (j = 0; j<nn; j++)
            {
                printf("marray[%d][%d]=%c\n",i,j,marray[i][j]);

            }//end of for j
        }//end of for i
}

Answer 1

There are several error in your code.

Don't cast malloc. And your are allocating the incorrect number of bytes both times. I advice you not to use sizeof(<type>), it's easy to make mistakes, it's better practice to call malloc like this:

int *arr = malloc(size * sizeof *arr);

Regardless of the type of arr, you would allocate the correct amount of memory.

You have to check if malloc returns NULL. If it returns NULL you cannot access the memory.

Also remember that a string must be '\0'-terminated, for a string of length n, you need n+1 bytes. You either allocate n+1 spaces for the strings, or in inputnames you should check if j < nn - 1. Also you don't set the '\0'-terminating byte in inputnames.

The correct allocation is:

char** p;
// calloc initializes the memory to 0,
// great initialization, helps you free the memory faster
// on error
p = calloc(m, sizeof *p);
if(p == NULL)
{
    fprintf(stderr, "not enough memory\n");
    return 1;
}

for (size_t i = 0; i < m; i++)
{
    p[i] = calloc(1, n+1); // size of char is 1
    if(p[i] == NULL)
    {
        fprintf(stderr, "not enough memory\n");
        free_strings(p, m);
        return 1;
    }
}

And you would need the free_strings function:

void free_strings(char **strings, size_t len)
{
    if(strings == NULL)
        return;

    for(size_t i = 0; i < len; ++i)
        free(strings[i]);
    free(strings);
}

Also your reading is not that efficient, I would use fgets instead:

int inputnames(char** marray, size_t mm, size_t nn)
{
    if(marray == NULL || mm == 0 || nn == 0)
        return 0;

    for(size_t i = 0; i < mm; ++i)
    {
        printf("Enter text #%d: ", i+1);
        fflush(stdout);

        if(fgets(marray[i], nn, stdin) == NULL)
        {
            fprintf(stderr, "cannot read anymore\n");
            return 0;
        }

        // removing the newline
        marray[i][strcspn(marray[i], "\n")] = 0;
    }

    return 1;
}

I forgot to mention this in the answer:

for (i = 0; i<mm; i++)
{
    while (marray[i][j] != '\n' && j<nn)
    {
        scanf_s("%c", &marray[i][j]);
    }
}//end of for i

This yields undefined behaviour. You only allocate memory but you don't initialize it, so you are actually checking if random value if equals to the newline. You also don't increment j, you are writing always at the same place. You need to read first, then check second.

for(i = 0; i<mm; i++)
{
    do {
        if(scanf_s("%c", marray[i] + j) != 1)
        {
            fprintf(stderr, "Unable to read from stdin\n");
            marray[i][j] = 0;
            return;
        }
    } while(marray[i][j++] != '\n' && j<nn);

    marray[i][j-1] = 0; // setting the \0-terminating byte
}

and the printitems should look like this:

void printitems(char** marray, size_t mm)
{
    if(marray == NULL || mm == 0 || nn == 0)
        return;

    for(size_t i = 0; i < mm; ++i)
        puts(marray[i]);
}

Also don't forget to free the memory when you don't need it anymore.