I have a function which returns a multiple indirection pointer as result like so:
typedef struct {
int id;
char *name;
} user;
user **myfn(int users_count) {
user **a;
a = malloc(sizeof(user) * user_counts);
for(int i = 0 ; i<user_counts ; i++) {
*(a+i) = malloc(sizeof(user));
(*(a+i))->id = 1;
(*(a+i))->name = malloc(sizeof(char) * 25);
strncpy((*(a+i))->name, "Morteza", 25); // just for example
}
return a;
}
Now, I when I want to crawl in this result in main function, it will show all users name, but at the end it will encounter with Segmentation fault
error.
int main() {
user **a = myfn(10);
int i = 0;
while((*(a+i)) != NULL) {
printf("ID: %d \t %s\n", (*(a+i))->id, (*(a+i))->name);
i++;
}
}
Results:
ID: 1 Morteza
ID: 2 Morteza
...
...
ID: 10 Morteza
Segmentation fault (core dumped)
Why condition
of while
doesn't work fine?
First of all,
a = malloc(sizeof(user) * user_counts);
has a problem - you want to allocate user_counts
instances of pointers to user
, not instances of user
, so that line should be
a = malloc(sizeof(user *) * user_counts);
However, there's an easier way around this - the sizeof
operator can take expressions as arguments as well as type names. So you can rewrite that line as
a = malloc( sizeof *a * user_counts );
The expression *a
has type user *
, so sizeof *a
is equivalent to sizeof (user *)
. This makes your life a bit simpler, in that you don't have to puzzle out the type that a
points to - make the compiler do the hard work.
You should always check the result of a malloc
call.
a = malloc( sizeof *a * users_count );
if ( a )
{
// do stuff with a
}
Second of all, don't use *(a+i)
to index into a
- use a[i]
instead. Makes things a bit easier to read. So,
*(a+i) = malloc(sizeof(user));
(*(a+i))->id = 1;
(*(a+i))->name = malloc(sizeof(char) * 25);
becomes
a[i] = malloc(sizeof *a[i]);
if ( a[i] )
{
a[i]->id = 1;
a[i]->name = malloc(sizeof *a[i]->name * 25);
}
else
{
/* deal with memory allocation failure */
}
Now, to your actual problem. Your code is crashing for one of the following reasons:
malloc
of a
failed, so you're crashing on the first line that uses a[i]
;malloc
of one of your a[i]
failed, so you're crashing on a[i]->id = 1
;users_count
elements of a
- no a[i]
is NULL
, so you loop past the last element of your array and try to dereference the object immediately following it, which is most likely not a valid pointer.In addition to adding the checks after each malloc
call, you should probably loop based on users_count
:
for ( i = 0; i < users_count; i++ )
printf("ID: %d \t %s\n", a[i]->id, a[i]->name);
Or, you need to allocate one extra element for a
and set it to NULL
:
a = malloc( sizeof *a * (users_count + 1) );
if ( a )
{
a[users_count] = NULL;
for ( i = 0; i < users_count; i++ )
...
}
Note that calloc
initializes all allocated memory to 0, so you can use that and not worry about explicitly setting an element to NULL
:
a = calloc( users_count + 1, sizeof *a );
if ( a )
{
for ( i = 0; i < users_count; i++ )
...
}
Collected from the Internet
Please contact [email protected] to delete if infringement.
Comments