You first need to understand that there is no Array present in the entirety of the code you have posted. Your struct and its members are pointers to blocks of memory you must allocate and manage.
Your logic for your while (ch ...)
loop is jumbled leading confusing expressions like:
(A[i].num)--; /* we have increased the number of neighbors one time more */
/* than the actal size */
And then multiple attempted reallocations without a single allocation/reallocation validation or any validation of input.
There is no need to cast the return of malloc
, it is unnecessary. See: Do I cast the result of malloc?. When setting the size for your allocation, if you use the dereferenced type, you will always get your type size correct.
...
while (1) {
/*allocation for colour, name, */
A[i].col = malloc (16 * sizeof *A[i].col);
A[i].name = malloc (16 * sizeof *A[i].name);
A[i].nb = malloc (sizeof *A[i].nb);
A[i].nb[A->num] = malloc (sizeof *A[i].nb[A[i].num]);
/* validate EVERY allocation */
if (!A[i].col || !A[i].name || !A[i].nb || !A[i].nb[A->num]) {
perror ("malloc-col,name,nb");
return 1;
}
/* validate EVERY input - protect array bounds */
if (scanf ("%15s %15s", A[i].col, A[i].name) != 2) {
fputs ("error: failed to read col & name.\n", stderr);
return 1;
}
...
(note: you must validate EVERY allocation and EVERY reallocation, just as you must validate EVERY input)
Note the use of the field-width modifier to limit the read of character to 15 saving +1 for the nul-terminating character in
/* validate EVERY input - protect array bounds */
if (scanf ("%15s %15s", A[i].col, A[i].name) != 2) {
fputs ("error: failed to read col & name.\n", stderr);
return 1;
}
Further, you ALWAYS realloc
using a temporary pointer. When realloc
fails, it returns NULL
and if fail to use a temporary pointer, you have just overwritten your pointer to your original block of memory with NULL
creating a memory-leak because you have lost your reference to the original block which now cannot be freed. Instead, you do something like:
/* ALWAYS realloc with a temprary pointer */
void *tmp = realloc (A[i].nb, (A[i].num + 1) * sizeof *A[i].nb);
if (!tmp) { /* validate EVERY reallocation */
perror ("realloc-A[i].nb");
return 1;
}
A[i].nb = tmp; /* assign new block to A[i].nb after validation */
For your control of your loop checking for '\t'
and ' '
, you can make the logic simpler by incorporating your EOF
check in the loop itself and then realloc
A
after, e.g.
/* see if there are neighbors or next country or the end*/
while ((ch = getchar()) != EOF && (ch == '\t' || ch == ' ')) {
if (scanf("%15s", A[i].nb[A[i].num]) != 1) {
fputs ("error: EOF of read of A[i].nb[A[i].num].\n", stderr);
return 1;
}
A[i].num++;
/* ALWAYS realloc with a temporary pointer */
void *tmp = realloc (A[i].nb, (A[i].num + 1) * sizeof *A[i].nb);
if (!tmp) { /* validate EVERY reallocation */
perror ("realloc-A[i].nb");
return 1;
}
A[i].nb = tmp; /* assign new block to A[i].nb after validation */
A[i].nb[A[i].num] = malloc (16 * sizeof *A[i].nb[A[i].num]);
if (!A[i].nb[A[i].num]) {
perror ("malloc-A[i].nb[A[i].num]");
return 1;
}
}
/* realloc A */
void *tmp = realloc (A, (i + 1) * sizeof *A);
if (!tmp) { /* validate */
perror ("realloc-A");
return 1;
}
A = tmp; /* assign reallocated block */
i++; /* increment counter ('ncountry' better choice than i?) */
After going back through your example, my best guess at what you are trying to accomplish is contained in the following example. If you provide a bit of sample input I can validate further.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
struct country {
char *name; /* generally the '*' goes with the variable not the type */
char *col; /* char* a, b, c; does NOT create 3 pointers to char... */
int num; /* char *a, b, c; makes that clear */
char **nb;
struct country *add;
};
int main (void) {
int ch, i = 0;
struct country *A = malloc (sizeof *A); /* allocate 1 struct */
if (!A) { /* validate EVERY allocation */
perror ("malloc-A");
return 1;
}
A[i].num = 0;
while (1) {
/*allocation for colour, name, */
A[i].col = malloc (16 * sizeof *A[i].col);
A[i].name = malloc (16 * sizeof *A[i].name);
A[i].nb = malloc (sizeof *A[i].nb);
A[i].nb[A->num] = malloc (sizeof *A[i].nb[A[i].num]);
/* validate EVERY allocation */
if (!A[i].col || !A[i].name || !A[i].nb || !A[i].nb[A->num]) {
perror ("malloc-col,name,nb");
return 1;
}
/* validate EVERY input - protect array bounds */
if (scanf ("%15s %15s", A[i].col, A[i].name) != 2) {
fputs ("error: failed to read col & name.\n", stderr);
return 1;
}
/* see if there are neighbors or next country or the end*/
while ((ch = getchar()) != EOF && (ch == '\t' || ch == ' ')) {
if (scanf("%15s", A[i].nb[A[i].num]) != 1) {
fputs ("error: EOF of read of A->nb[A->num].\n", stderr);
return 1;
}
A[i].num++;
/* ALWAYS realloc with a temprary pointer */
void *tmp = realloc (A[i].nb, (A[i].num + 1) * sizeof *A[i].nb);
if (!tmp) { /* validate EVERY reallocation */
perror ("realloc-A->nb");
return 1;
}
A[i].nb = tmp; /* assign new block to A->nb after validation */
A[i].nb[A[i].num] = malloc (16 * sizeof *A[i].nb[A[i].num]);
if (!A[i].nb[A[i].num]) {
perror ("malloc-A[i].nb[A[i].num]");
return 1;
}
}
/* realloc A */
void *tmp = realloc (A, (i + 1) * sizeof *A);
if (!tmp) { /* validate */
perror ("realloc-A");
return 1;
}
A = tmp; /* assign reallocated block */
i++; /* increment counter ('ncountry' better choice than i?) */
}
}
Look things over and let me know if you have further questions.