Add data dynamically to array of pointers to structures within function

Question

I tried many combinations but really nothing worked. It's been long enough so I decided to write this issue.

I just want an array of pointers to structures so I could later easiely sort it by swaping the addresses. I have a function to get data from file and write to array. Unfortunately, it's impossible for me to read this data outside the function.

My last attempt (I deleted file operations as those are not the issue):

Header.h:

#pragma once

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

typedef struct {
    int32_t year;
    uint16_t month;
    uint16_t day;
} Date;

typedef struct {
    char name[16];
    Date date;
    uint32_t number;
} Player;

size_t readData(Player** players);

Source.c:

#include "Header.h"

size_t readData(Player** players) {
    players = NULL;

    players = realloc(players, sizeof(Player*) * 1);
    players[0] = malloc(sizeof(Player));
    strcpy(players[0]->name, "asd");
    printf("$ %s\n", players[0]->name);//OK

    return 1;
}

int main() {
    Player **players = NULL;
    uint32_t elemCount, i;

    elemCount = readData(players);

    printf("> %s", players[0]->name);//BUM - ERROR!

    return 0;
}

I'm using Visual Studio Community 2015. I know it's not so much for coding in C but I managed to configure the solution and it doesn't seem to be the thing imo.

I will be very thankful for help. Yet, I'd like to see it as a remake of my code. I tried some other answers to questions on StackOverFlow like this but they didn't help.

when calling realloc(), always save the returned value into a temporary variable, then check (!=NULL) that variable before assigning to the target variable. That way, when realloc() fails, the original allocated memory pointer is not lost then if == NULL, then handle error — user3629249
– user3629249, Commented Jan 16, 2016 at 14:27
this line: players = NULL; is a very bad idea as the passed pointer is being destroyed — user3629249
– user3629249, Commented Jan 16, 2016 at 14:28
the function: readData() is returning a size_t but that size_t is being assigned to a uint32_t. this results in an implicit conversion. which can result is the value being corrupted. Suggest: change the signature of the readData() function to return a uint32_t — user3629249
– user3629249, Commented Jan 16, 2016 at 14:35
this line: int32_t year; should probably be uint32_t year; as there are no negative years — user3629249
– user3629249, Commented Jan 16, 2016 at 14:51
when compiling, always enable all the warnings, then fix those warnings. (for gcc, at a minimum use: -Wall -Wextra -pedantic (I also recommend: -Wconversion -std=c99) ). Amongst other things, this will show that the variable i is unused. — user3629249
– user3629249, Commented Jan 16, 2016 at 15:12

Rabbid76 · Accepted Answer · 2016-01-15 18:20:09Z

2

If a parameter of a function is not only read and should be an output too, you have to pass a pointer to your data to the function. If your data type is Player** your paramter type has to be Player ***, so the list of players itselfe is able to be changed inside the function.

size_t readData(char* fname, Player*** players) {
                                // ^ players is a input and output parameter
    Player **tempPlayers = *players; // local list of players
    tempPlayers = realloc(tempPlayers, sizeof(Player*) * 1);
    tempPlayers[0] = malloc(sizeof(Player));
    strcpy(tempPlayers[0]->name, "asd");
    printf("$ %s\n", tempPlayers[0]->name);//OK

    *players = tempPlayers; // write back list of players to paramter
    return 1;
}

int main() {
    Player **players = NULL;
    uint32_t elemCount, i;

    char *fileName = NULL; 
    elemCount = readData(&players);
                      // ^

    printf("> %s", players[0]->name);//BUM - ERROR!

    return 0;
}

If you don't want to use *** you can do it like this:

Player* *readData(char* fname, Player** players, size_t *size) {
    players = realloc(players, sizeof(Player*) * 1);
    players[0] = malloc(sizeof(Player));
    strcpy(players[0]->name, "asd");
    printf("$ %s\n", players[0]->name);//OK

    *size = 1;
    return players;
}

players = readData( fileName,  players, &elemCount );

edited Jan 15, 2016 at 18:20

answered Jan 15, 2016 at 17:54

Rabbid76

212k30 gold badges163 silver badges205 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

user3629249 Over a year ago

this answer contains two potential problems. 1) uses the returned value from realloc() without first checking that the call to realloc() was successfu;. 2) uses the returned value from malloc() without first checking that the call to malloc() was successful.

user4520 · Accepted Answer · 2016-01-15 18:14:42Z

1

You're passing the pointer to the function by value; the original players in main is not changed. This is no different from doing:

#include <stdio.h>

void func(int x)
{
    x = 4;
}

int main()
{
    int x = 0;
    func(x);

    printf("%d\n", x);      // zero
}

I'm assuming it's the pointers that got you confused, so you should see your mistake now.

To make the modification visible outside of readData, you will need to use three levels of indirection:

size_t readData(Player*** players) {
    *players = malloc(sizeof(Player*) * 1);
    *(players)[0] = malloc(sizeof(Player));
    strcpy((*players)[0]->name, "asd");
    printf("$ %s\n", (*players)[0]->name);//OK

    return 1;
}

int main() {
    Player **players = NULL;
    uint32_t elemCount, i;

    elemCount = readData(&players);

    printf("> %s", players[0]->name);  // prints 'asd'

    return 0;
}

There is no point using realloc when you know the pointer being passed is always NULL, so I've changed it to malloc instead.

And on a side note: Visual Studio is perfectly suited to C development, and implements parts of C99 and even C11 (historically MSVC was always stuck at C89). You can just use the .c extension for your source files, and this will make the compiler assume the code is C, or you can explicitly set this in the property pages.

edited Jan 15, 2016 at 18:14

answered Jan 15, 2016 at 17:56

user4520

3,4951 gold badge33 silver badges55 bronze badges

1 Comment

Nonemoticoner Over a year ago

Thank you for your contribution as well - I applauded ;)

user3629249 · Accepted Answer · 2016-01-16 15:05:11Z

1

parameter passed to readData() must be the address of the caller's pointer 'players'

each of the references in readData() to players must take that into account.

Othewise the caller's pointer 'player' will not be updated.

Then the main() function, call to printf() is trying to output from address 0, resulting in undefiined behaviour, leading to a seg fault event.

in the main() function, if you insert:

printf( "player value; %p\n", (void*)players);

before the current call to printf() you will see the players pointer still contains NULL.

answered Jan 16, 2016 at 15:05

user3629249

16.6k1 gold badge18 silver badges17 bronze badges

Collectives™ on Stack Overflow

Add data dynamically to array of pointers to structures within function

3 Answers 3

1 Comment

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related