Dynamic allocation of memory in C

Question

I need to store five names of songs in memory using dynamic allocation and then print them to the screen.

What is wrong with my code?

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

#define MAXIM 5

void main() {
    char song[30];
    char *p;

    p = (char*)calloc(MAXIM, sizeof(song) + 1);
    for (int i = 0; i < MAXIM; i++) {
        printf("Name of the song %d:\n", i);
        scanf("%s", song);
        strcpy(p[i], song);
    };
    for (int i = 0; i < MAXIM; i++) {
        printf("%c\n", p[i]);
        free(p[i]);
    }

     getch();
}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Nov 12 '16 at 13:22
The variable `p` is not an array of strings. It's a pointer to `char` so it can be *a* string, but not an array of strings. You seem to want an array of arrays of `char` (like `char p[MAXIM][30]`). Start with that and think about how to dynamically allocate the different parts. — Some programmer dude, Nov 12 '16 at 13:24
@SouravGhosh he used C++ as C (see visual-studio tag and `for (int i = 0;`) — BLUEPIXY, Nov 12 '16 at 13:30

chus · Accepted Answer · 2016-11-13T22:00:59.650

There are two mistakes that are corrected and explained in the following code. Besides, scanf() is replaced by fgets():

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

#define MAXIM 5
#define NAME_SIZE 30

int main(void) {
    char *p[MAXIM];  /* MAXIM pointers to char */

    for (int i = 0; i < MAXIM; i++) {
        p[i] = calloc(1, NAME_SIZE + 1);
        printf("Name of the song %d:\n", i);
        /* reads a maximum of NAME_SIZE chars including trailing newline */
        fgets(p[i], NAME_SIZE+1, stdin);   
        /* removes trailing newline */
        p[i][strcspn(p[i], "\r\n")] = 0;
    }

    for (int i = 0; i < MAXIM; i++) {
        printf("%s\n", p[i]);  /* %s format specifier */
        free(p[i]);
    }
    getch();
    exit(0);
}

Potential buffer overflow in `scanf("%s", p[i])` – chqrlie Nov 13 '16 at 12:39 — chqrlie, Nov 13 '16 at 12:39
@chqrlie You're right. I'll update my answer. – chus Nov 13 '16 at 20:36 — chus, Nov 13 '16 at 20:36

chqrlie · Answer 2 · 2016-11-13T22:10:37.910

You allocate a 2D array of char but use a simple pointer to char to access it. Use the proper pointer type: pointer to an array of arrays of 30 char, namely char (*p)[30]; :

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

#define MAXIM 5

int main(void) {
    char song[30];
    char (*p)[30];
    int n;

    p = calloc(MAXIM, sizeof(*p));
    for (n = 0; n < MAXIM; n++) {
        printf("Name of the song %d:\n", n);
        if (fgets(p[n], sizeof(p[n]), stdin) == NULL)
            break;
        p[n][strcspn(p[n], "\n")] = '\0'; // strip the trailing newline
    }
    for (int i = 0; i < n; i++) {
        printf("%s\n", p[i]);
    }
    free(p);

    getch();
    return 0;
}

Notes:

scanf() should be given the maximum number of characters to read into the destination array.
free() the allocated pointer, not individual subarrays.
main() returns an int.

It does not work if the name of a song contains a white space since `sscanf("%29s", p[i])` stops at a whitespace. — chus, Nov 13 '16 at 22:05
@chus: that's true. The OP's code has this shortcoming too. Your solution handles this correctly. I shall update my code. — chqrlie, Nov 13 '16 at 22:08

Dynamic allocation of memory in C

2 Answers2