You code has a number of issues, but let's back off of that for a minute and talk about memory management in general.
A common (and my preferred) idiom for allocating memory is something like
T *p = malloc( sizeof *p * N );
or
T *p = NULL;
...
p = malloc( sizeof *p * N );
which will allocate enough space to hold N elements of type T
. Note that there is no cast on the result of malloc
; as of the 1989 standard, the cast is no longer necessary1, and under C89 compilers it will suppress a diagnostic if you don't have a declaration for malloc
in scope (i.e., you forgot to include stdlib.h
). C99 got rid of implicit int
declarations, so that's not as big of an issue anymore, but a lot of compilers default to the 1989 standard.
Note that I also use sizeof *p
to get the size of each element; the expression *p
has type T
, so it follows that sizeof *p
is equivalent to sizeof (T)
2. Not only does this reduce visual clutter, it saves you a minor maintenance headache of the type of *p
ever changes, say from int
to long
or float
to double
.
C isn't your mother and won't clean up after you; you are responsible for deallocating any memory you allocated when you are done with it.
Note that you can resize a dynamically-allocated buffer with the realloc
library function. Here's a typical usage:
#define INITIAL_SIZE ... // some initial value
...
size_t arraysize = INITIAL_SIZE;
size_t counter = 0;
T *array = malloc( sizeof *array * arraysize );
...
while ( we_need_to_store_more_data )
{
if ( counter == arraysize )
{
/**
* We've run out of room to store new things, so
* we need to extend the array; one common
* strategy is to double the size of the array
* each time.
*/
T *tmp = realloc( sizeof *array * (2 * arraysize) );
if ( tmp )
{
array = tmp;
arraysize *= 2;
}
}
array[counter++] = new_value;
}
Note that for a type like
struct foo
{
char *name;
char *address;
int value;
};
you have to allocate space for name
and number
separately from the instance of struct foo
itself:
struct foo *farr = malloc( sizeof *farr * N );
...
farr[i].name = malloc( sizeof *farr[i].name * name_length );
farr[i].address = malloc( sizeof *farr[i].address * address_length);
You have to be really careful when you get into multiple allocation and deallocation steps like this; it's a really good idea to abstract these operations out into their own functions, like:
if ( !add_data( &foo[i], newName, newAddress, newValue ))
// report error
which would look something like
int add_data( struct foo *elem, const char *name, const char *addr, int val )
{
assert( elem != NULL );
elem->name = malloc( strlen( name ) + 1 );
elem->address = malloc( strlen( addr ) + 1 );
if ( !elem->name || !elem->address ) // Make sure memory allocation
{ // succeeded before trying to
free( elem->name ); // use name or address; malloc returns
free( elem->address ); // NULL on failure, and free(NULL)
return 0; // is a no-op. We want to undo any
} // partial allocation before returning
// an error.
strcpy( elem->name, name );
strcpy( elem->address, addr );
elem->value = val;
return 1;
}
Now let's look at the specific issues in your code:
train_array = (struct train*)malloc(sizeof(struct train_array)*rows);
^^^^^^^^^^^^^^^^^^^
The name of the type is struct train
; the name of the object you're trying to allocate is train_array
. You've crossed the two, which is a problem. Going by my advice above, rewrite that to
train_array = malloc( sizeof *train_array * rows );
This line
train_array[i] = (struct train*)malloc(sizeof(struct person_array));
has two problems. You've already allocated train_array[i]
; what you need to allocate is space for the start
and destination
elements within each train_array[i]
. Secondly, you haven't defined person_array
anywhere in your code, which is why the compiler is complaining about a non-existent type.
You don't need to loop through the file twice; you can extend your array as you go as I demonstrated above. Here's an example of how you could restructure your code:
int done = 0;
...
size_t rows = 0;
size_t totalRows = 16; // initial array size
train_array = malloc( sizeof *train_array * totalRows ); // initial allocation
while ( scanf( "%s %s %d %d", temp_station,
temp_destination,
&time_depature,
&time_arrival) == 4 )
{
if ( rows == totalRows )
{
struct train *tmp = realloc( sizeof *train_array * ( 2 * totalRows ));
if ( tmp )
{
train_array = tmp;
totalRows *= 2;
}
else
{
fprintf( stderr, "could not extend train_array past %zu elements\n", totalRows);
break;
}
}
train_array[rows].start_time = time_departure;
train_array[rows].arrival_time = time_arrival;
train_array[rows].start = malloc( strlen( temp_station ) + 1 );
if ( train_array[rows].start )
strcpy( train_array[rows].start, temp_station );
train_array[rows].end = malloc( strlen( temp_destination ) + 1 );
if ( train_array[rows].end )
strcpy( train_array[rows].destination, temp_destination );
}
Note that when you're done with this memory, you'd deallocate it as follows:
for ( size_t i = 0; i < totalRows; i++ )
{
free( train_array[i].start );
free( train_array[i].end );
}
free( train_array );
1. In C, values of type void *
can be assigned to any other pointer type without a cast. This is not true in C++; under a C++ compiler a cast would be required, but if you're writing C++ you should be using new
and delete
instead of malloc
and free
.
2. sizeof
is an operator, not a function; you only need to use parentheses if the operand is a type name like int
or float *
.