1

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.

Nonemoticoner
  • 650
  • 5
  • 14
  • 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 Jan 16 '16 at 14:27
  • this line: `players = NULL;` is a very bad idea as the passed pointer is being destroyed – user3629249 Jan 16 '16 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 Jan 16 '16 at 14:35
  • this line: `int32_t year;` should probably be `uint32_t year;` as there are no negative years – user3629249 Jan 16 '16 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 Jan 16 '16 at 15:12

3 Answers3

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 );
Rabbid76
  • 202,892
  • 27
  • 131
  • 174
  • 1
    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. – user3629249 Jan 16 '16 at 15:10
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.

user4520
  • 3,401
  • 1
  • 27
  • 50
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.

user3629249
  • 16,402
  • 1
  • 16
  • 17