2

I'm trying to dynamically allocate an array of structs by passing the pointer to a function. I need to access the array using indexing. I've got a similar proccess working without passing to a function. I have a simple struct called Account that only has one member, accountNo. Listed below is the relevant malloc call

int8_t dynamicStruct(struct Account **all_accounts,int8_t num_accounts){
    *all_accounts = (struct Account*)malloc(sizeof(struct Account)*num_accounts);
}

The variable all_accounts is initialized and called with the following snippet, with num_accounts at this point being 10;

struct Account *all_accounts_dyn;
dynamicStruct(&all_accounts_dyn,num_accounts);

Accessing the member variable accountNo with the following method

all_accounts[i]->accountNo = i;

The program compiles fine, manages to allocate the memory, but segfaults upon accessing a member (num_accounts = 10).

Compiled with

gcc -std=gnu99 -Wall -Werror structify.c -o structify

"Small self contained example"

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <stdbool.h>

struct Account{
    int8_t accountNo;
};
int8_t dynamicStruct(struct Account **all_accounts,int8_t num_accounts);

int main(){
    struct Account *all_accounts_dyn;
    int8_t num_accounts = 10;
    dynamicStruct(&all_accounts_dyn,num_accounts);
    return 1;
}
int8_t dynamicStruct(struct Account **all_accounts,int8_t num_accounts){
        *all_accounts = (struct Account*)malloc(sizeof(struct Account)*num_accounts);
        for(int i = 0; i<num_accounts;i++){
            printf("initializing %d\n",i);
            all_accounts[i]->accountNo = i;
        }
        return 1;
}
  • Post a minimal self contained example please. – 2501 Oct 18 '16 at 07:20
  • Are you saying that you have allocated for 10 `Account` structs, and are accessing with `all_accounts[10]`, which would of course be out of bounds? – ad absurdum Oct 18 '16 at 07:27
  • No, I'm saying if I try to access a member variable, it segfaults, not 10 specifically. – NathanielJPerkins Oct 18 '16 at 07:30
  • Your example does not compile. – 2501 Oct 18 '16 at 07:32
  • You don't have a array of structs in your program. It is just a plain pointer to a struct and not a array of structs thats why it segfaults. – ckruczek Oct 18 '16 at 07:37
  • 1
    `all_accounts[i]->accountNo = i;` --> `(*all_accounts)[i].accountNo = i;` – LPs Oct 18 '16 at 07:44
  • Isn't -> notation short hand for (*struct). notation? why are they not interchangeable here? – NathanielJPerkins Oct 18 '16 at 07:46
  • all_accounts[x] is a pointer to that struct is it not? – NathanielJPerkins Oct 18 '16 at 07:47
  • `all_accounts[0]` is a pointer to `all_accounts_dyn`, but major indices are UB... – LPs Oct 18 '16 at 07:49
  • major indices are what? UB? – NathanielJPerkins Oct 18 '16 at 07:51
  • @NathanielJPerkins [**U**ndefined **B**ehavior](https://en.wikipedia.org/wiki/Undefined_behavior) – LPs Oct 18 '16 at 08:09
  • In C, The heap memory allocation functions (malloc, calloc, realloc) return a value with type `void*` Such type can be assigned to any other pointer. Casting the returned type just clutters the code, making it more difficult to understand, debug, maintain. Strongly suggest removing the cast of the value returned from a call to any of these functions. When calling any of the memory allocation functions, always check (!=NULL) the returned value to assure the operation was successful. – user3629249 Oct 20 '16 at 12:42
  • regarding the returned value from the `main()` function. Any value other than 0 indicates a failure. Strongly suggest (in the main() function) replace `return 1;` with `return 0` (or on modern compilers, do not use any `return 0;` and the compiler will have the `main()` function return a 0 – user3629249 Oct 20 '16 at 12:44
  • I compile with -Wall and -Werror, it wouldn't let me if I wanted to. Though I don't think casting with the malloc makes it harder to understand, debug or maintain at all. – NathanielJPerkins Oct 20 '16 at 12:45
  • for ease of readability and understanding, 1) consistently indent the code. never use tabs as every word-processor/editor has the tab stops/tab width set for individual preferences. – user3629249 Oct 20 '16 at 12:50
  • when compiling, always enable all the warnings, then fix those warnings. (for `gcc`, at a minimum, use: `-Wall -Wextra -pedantic` I also use: `-Wconversion -std=gnu99` ). – user3629249 Oct 20 '16 at 12:53
  • the function: `dynamicStruct()` returns an `int8_t` of some wierd value of 1 and the caller does not check that value. Suggest changing the returned type to `void` – user3629249 Oct 20 '16 at 12:54
  • Now you're really just jumping into programming preference and style guide, which is neither relevant, nor asked for. This wasn't a critique of my usage of tabs or spaces or properly checking NULL allocation. It was asking about a very specific problem, with absolutely everything irrelevant stripped from the code. There's a reason you don't see me checking NULL or returning anything else, it doesn't matter to the problem I asked about. – NathanielJPerkins Oct 20 '16 at 12:58
  • As a general principal, do not `#include` header files that are not being used. – user3629249 Oct 20 '16 at 13:05

4 Answers4

2

When you have struct my_struct;, you access the members like this:

mystuff = my_struct.stuff;

But when you have a pointer to a struct struct *my_struct;, you do:

mystuff = my_struct->stuff;

This is equivalent to:

mystuff = (*my_struct).stuff;

In other words, you have to dereference the pointer to struct before you can access its member fields.

When you allocate memory for 10 structs:

*all_accounts = (struct Account*)malloc(sizeof(struct Account)*num_accounts);

and then access with array notation, you have all_accounts[i] ==> *(all_accounts + i). So, all_accounts[i] is a struct, not a pointer to a struct, and must be accessed with the dot operator.

ad absurdum
  • 19,498
  • 5
  • 37
  • 60
  • Thanks. That was my main problem. I thought the all_accounts[i] returned a pointer to a struct rather than a struct. The dot notation makes more sense now. – NathanielJPerkins Oct 18 '16 at 07:58
2

As you wrote, to allocate space for all_accounts_dyn you used

*all_accounts = malloc(sizeof(struct Account)*num_accounts);

It is correct due to the double pointer.

The double pointer takes the address of pointer all_accounts_dyn

The returned address of malloc is assigned to all_accounts_dyn that is *all_accounts inside the dynamicStruct function.

So when you loop to init the struct you have to first dereference the double pointer and the dereference all_accounts_dyn items.

    for(int i = 0; i<num_accounts;i++)
    {
        (*all_accounts)[i].accountNo = i;

        printf("initialized %d\n", (*all_accounts)[i].accountNo);

    }
LPs
  • 16,045
  • 8
  • 30
  • 61
-1

If you are trying to allocate an araay of structs,then why are you using double pointers.

all_acounts=(Acount*)malloc(sizeof(Acount)*num_acounts) and in the function dynamicStruct set the argument as normal non double pointer *all_acounts

JPX
  • 93
  • 1
  • 4
  • 2
    http://stackoverflow.com/questions/5580761/why-use-double-pointer-or-why-use-pointers-to-pointers Second answer: Use ** when you want to preserve (OR retain change in) the Memory-Allocation or Assignment even outside of a function call. – NathanielJPerkins Oct 18 '16 at 07:40
  • This doesn't answer the question. If you don't understand something in the question, please refrain from posting an answer. Instead, ask a new question. – Lundin Oct 18 '16 at 09:55
  • This does not provide an answer to the question. Once you have sufficient [reputation](http://stackoverflow.com/help/whats-reputation) you will be able to [comment on any post](http://stackoverflow.com/help/privileges/comment); instead, [provide answers that don't require clarification from the asker](http://meta.stackexchange.com/questions/214173/why-do-i-need-50-reputation-to-comment-what-can-i-do-instead). - [From Review](/review/low-quality-posts/14016370) – ljedrz Oct 18 '16 at 11:48
-1

the posted code contains several problems.

  1. incorrect return value from main()
  2. unused return value from dynamicStruct()
  3. using wrong type when passing num_accounts
  4. needs appropriate vertical and horizontal spacing, for readability
  5. does not properly set the values in the array of structs
  6. fails to check for errors in call to malloc()
  7. contains several risky conversions between different numeric types
  8. contains tabs for indenting the code
  9. does not indent the code consistently
  10. returns a misleading value from the main() function
  11. casts the returned value from malloc(), which is a bad practice in C

and now the corrected code:

#include <stdio.h>   // printf()
#include <stdint.h>  // int8_t 
#include <stdlib.h>  // malloc(), exit(), EXIT_FAILURE
//#include <stdbool.h>

struct Account
{
    int8_t accountNo;
};

void dynamicStruct( struct Account **all_accounts, size_t num_accounts );

int main( void )
{
    struct Account *all_accounts_dyn;
    size_t num_accounts = 10;
    dynamicStruct( &all_accounts_dyn, num_accounts );
} // end function: main


void dynamicStruct( struct Account **all_accounts, size_t num_accounts )
{
        *all_accounts = malloc(sizeof(struct Account)*num_accounts);
        if( NULL == *all_accounts )
        {
            perror( "malloc for account array failed" );
            exit( EXIT_FAILURE );
        }

        // implied else, malloc successful


        for( int8_t i = 0; i<(int8_t)num_accounts; i++ )
        {
            printf("initializing %d\n",i);
            (*all_accounts)[i].accountNo = i;
        }
} // end function: dynamicStruct

The above code outputs the following:

initializing 0
initializing 1
initializing 2
initializing 3
initializing 4
initializing 5
initializing 6
initializing 7
initializing 8
initializing 9
user3629249
  • 16,402
  • 1
  • 16
  • 17