Reputation:
How can I fix this code in a way that it prints the words in the array? Moreover this is the right way to dynamically allocate memory for n
words of max size 40?
int main() {
int n;
char *arr;
int i;
printf("Give me a number:");
scanf("%d", &n);
arr = malloc(n * 40);
for (i = 0; i < n; i++)
{
printf("Give me a word: ");
scanf("%s", &arr[i]);
}
for (i = 0; i < n; i++)
{
printf("%s", arr[i]); //< --problem here
}
return 0;
}
Upvotes: 1
Views: 2049
Reputation: 23832
Your allocation is not the best, and printf
argument arr[i]
expects a char*
but you pass it an int
(a char
if you'd like).
Here is how you should do it, with comments:
#include <stdio.h>
#include <stdlib.h> //for malloc
int main(){
int n;
int i;
printf("Give me a number:");
scanf("%d", &n);
//declare a variable of pointer to pointer to char
//allocate memory for the array of pointers to char,
//each one capable of pointing to a char array
char **arr = malloc(n * sizeof *arr);
if(arr == NULL){ //check for allocation errors
perror("malloc");
return EXIT_FAILURE;
}
//allocate memory for each individual char array
for(i = 0; i < n; i++){
arr[i] = malloc(40); //char size is always 1 byte
if(arr == NULL){ //check for allocation errors
perror("malloc");
return EXIT_FAILURE;
}
}
for (i = 0; i < n; i++){
printf("Give me a word: ");
//limit the size of read input to 39 charaters to avoid overflow,
//a nul character will be added by scanf
scanf("%39s", arr[i]);
}
for (i = 0; i < n; i++){
printf("%s\n", arr[i]);
}
for(int i = 0; i < n; i++){ //free the memory for each char array
free(arr[i]);
}
free(arr); //free array of pointers
return 0;
}
You can also do this with less code using a pointer to array of 40 chars, this will simplify the memory allocation and deallocation:
Sample with comments:
#include <stdio.h>
#include <stdlib.h> //for malloc
int main(){
int n;
int i;
printf("Give me a number:");
scanf("%d", &n);
//declare a pointer to array of chars and
//allocate memory for all the char arrays
char (*arr)[40] = malloc(n * sizeof *arr);
if(arr == NULL){ //check for allocation errors
perror("malloc");
return EXIT_FAILURE;
}
for (i = 0; i < n; i++){
printf("Give me a word: ");
scanf("%39s", arr[i]);
}
for (i = 0; i < n; i++){
printf("%s\n", arr[i]);
}
free(arr); //free allocated memory
return 0;
}
Upvotes: 2
Reputation: 498
This:
for(i=0;i<n;i++){
printf("Give me a word: ");
scanf("%s",&arr[i]);
}
is probably not what you want.
You probably want this instead:
for(i=0; i<n; i++){
printf("Give me a word: ");
scanf("%s", arr + i*40);
}
then later:
for(i=0; i<n; i++){
printf("%s", arr + i*40);
}
Remember that a string in C is just an array of characters.
Thus when defining char *arr
, you are creating a single string. Had you done char **arr
it would have been an array of strings, which is what you want.
However, I find that allocating/freeing arrays of arrays on the heap to be rather inconvenient, and prefer 'flattening' them into a single array.
This is exactly what you were doing with arr = malloc(n*40)
, but then later you treated this array of characters as an array of strings when you did this:
for(i=0; i<n; i++){
printf("Give me a word: ");
scanf("%s", &arr[i]);
}
Note that this is actually perfectly legal (scanf
wanted a char*
and you gave it one), but it's a logical error since you are giving it the n
-th character when you wanted to give it the n
-th array.
And oh yes, don't forget to free(arr)
later.
Upvotes: 1