How to make a function return a pointer to array struct

Question

I'm working on a function in C that should return a pointer to an array struct. The struct is

struct corr{
  char nome[128];
  char cognome[128];
  char password[10];
  float importo;
};
typedef struct corr correntista;

The function that return a pointer to this struct is

correntista *max_prelievo(FILE *fcorrentisti, FILE *fprelievi){
    correntista *corr_max[2];
    corr_max[0] = (correntista *)malloc(sizeof(correntista));
    corr_max[1] = (correntista *)malloc(sizeof(correntista));
    /*.........*/
    return *corr_max;
}

In the main program I want to print the returned value in the following way:

*c_max = max_prelievo(fc, fp);
printf("Correntista con max prelievi:\n");
printf("Nome: %s\n", c_max[0]->nome);
printf("Cognome: %s\n", c_max[0]->cognome);
printf("Max prelievo: %f\n\n", c_max[0]->importo);

printf("Correntista con max versamenti:\n");
printf("Nome: %s\n", c_max[1]->nome);
printf("Cognome: %s\n", c_max[1]->cognome);
printf("Max versamento: %f\n", c_max[1]->importo);

but only the first struct c_max[0] has the expected value. c_max[1] has garbage values. What should I change in my program?

What is the type of `c_max`? Try changing `correntista *max_prelievo(FILE *fcorrentisti, FILE *fprelievi){` to `correntista **max_prelievo(FILE *fcorrentisti, FILE *fprelievi){` and `return *corr_max;` to `return corr_max;`. and from the calling function, `correntista **c_max = max_prelievo(fc, fp);` — Spikatrix, Jun 10 '15 at 14:09
You don't need to cast the result of malloc - http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — Treesrule14, Jun 10 '15 at 14:11
@CoolGuy returning a pointer into an array that isn't guaranteed to exist once the function returns is obviously UB. — autistic, Jun 10 '15 at 14:21
@undefinedbehaviour , The array is gone, but since it was allocated with `malloc` and had not been freed, accessing it isn't UB, right? — Spikatrix, Jun 10 '15 at 14:23
@CoolGuy Having a pointer to pointer and returning it doesn't lead to UB.. — Gopi, Jun 10 '15 at 14:24
@CoolGuy I am obliged to define the function as correntista *max_prelievo(FILE *fcorrentisti, FILE *fprelievi). c_max type is "correntista" as the struct — mbistato, Jun 10 '15 at 14:25
@CoolGuy The array is blown. The array items were allocated by `malloc`, but that's irrelevant because it's a pointer to the first element of the array that was returned and the array is blown. — autistic, Jun 10 '15 at 14:29
@undefinedbehaviour , I see. So will the allocated memory be freed once the array is lost or is it memory leak? — Spikatrix, Jun 10 '15 at 14:32
@CoolGuy The biggest concern is that the behaviour is undefined, and the nature of undefined behaviour is such that its consequences can't be defined. Typically, yes, the second object will leak because it's no longer accessible. — autistic, Jun 10 '15 at 14:35
@undefinedbehaviour , Oh ok. Thanks for the information! :-) — Spikatrix, Jun 10 '15 at 14:37

autistic · Answer 1 · 2015-06-10T14:44:57.443

There are several solutions, but they all require an overhaul of how your function is to operate. Perhaps the best solution is to consider how scanf works; It returns no objects, instead it operates solely on objects that are pointed to by parameters passed to it. Hence:

void max_prelievo(correntista *corr_max, FILE *fcorrentisti, FILE *fprelievi){
    /* NOTE: You should make a habit of separating your allocation from this logic;
     *       it makes debugging and testing so much easier... */

    corr_max[0] = (correntista) { .nome = "Michael",
                                  .cognome = "Anderson",
                                  .importo = 42.0 };

    corr_max[1] = (correntista) { .nome = "Undefined",
                                  .cognome = "Behaviour",
                                  .importo = -1.0 };


    /*.........*/
}

int main(void) {
    correntistia c_max[2] = { 0 };
    FILE *f1 = fopen(...), *f2 = fopen(...);

    max_prelievo(c_max, f1, f2);

    printf("Correntista con max prelievi:\n");
    printf("Nome: %s\n", c_max[0].nome);
    printf("Cognome: %s\n", c_max[0].cognome);
    printf("Max prelievo: %f\n\n", c_max[0].importo);

    printf("Correntista con max versamenti:\n");
    printf("Nome: %s\n", c_max[1].nome);
    printf("Cognome: %s\n", c_max[1].cognome);
    printf("Max versamento: %f\n", c_max[1].importo);
}

What scanf does return is a single integer, indicating success. Perhaps, if you are to use return values in the future, you could resort to using a similar pattern? Look at the bright side: There's no malloc or free here and no chance you could leak any memory; the code is as simple as it could possibly get.

Alternatively, as horrific as it seems, if you must resort to using such a pattern then less calls to malloc will serve you well. Consider something like this:

correntista *max_prelievo(FILE *fcorrentisti, FILE *fprelievi) {
    correntista *corr_max = malloc(2 * sizeof *corr_max);

    corr_max[0] = (correntista) { .nome = "Michael",
                                  .cognome = "Anderson",
                                  .importo = 42.0 };

    corr_max[1] = (correntista) { .nome = "Undefined",
                                  .cognome = "Behaviour",
                                  .importo = -1.0 };

    /*.........*/

    return corr_max;
}

The main entry point will be almost identical to the example I gave earlier... except for one thing: Don't forget to free that memory! :(

@CoolGuy Thanks for the edits :) my astigmatism is too horrific to notice that I accidentally typed `,` instead of `.`... — autistic, Jun 10 '15 at 14:38
I would also say that this is the preferred solution, as how and where the memory is allocated has nothing to do with the actual algorithm of that function. — Lundin, Jun 10 '15 at 14:39

score 1 · Accepted Answer · answered Jun 10 '15 at 14:21

1

Remember what you have is array of pointers so when you exit the function the array is out of scope. Have a pointer to pointer for this purpose as shown below.

correntista **max_prelievo(FILE *fcorrentisti, FILE *fprelievi){
    correntista **corr_max = malloc(sizeof(correntista *) * 2);
    corr_max[0] = malloc(sizeof(correntista));
    corr_max[1] = malloc(sizeof(correntista));
    /*.........*/
    return corr_max;
}

The call should be

correntista **c_max = max_prelievo(fc, fp);

answered Jun 10 '15 at 14:21

Gopi

19,784
4
24
36

This isn't correct though. A pointer to pointer is not a pointer to an array and you aren't allocating an array, you are allocating a [segmented heap fiasco](http://stackoverflow.com/a/30117625/584518), which is something you should never do. Instead, allocate an array. `correntista* corr_max = malloc(2 * sizeof(*corr_max));` or alternatively use an array pointer (although the syntax for returning array pointers from functions is very ugly). – Lundin Jun 10 '15 at 14:43
@user3700643 Indeed. Technically speaking, this doesn't meet your requirements as specified in your comment to Cool Guy: "I am obliged to define the function as correntista *max_prelievo(FILE *fcorrentisti, FILE *fprelievi). c_max type is "correntista" as the struct" – autistic Jun 10 '15 at 14:47
@Lundin I agree!! I don't know where I was when I wrote this code.. Just going through it again I feel pointer to pointer is unnecessary here.. Anyways even though this approach is ugly it is not wrong so I keep this answer :) – Gopi Jun 10 '15 at 15:37

How to make a function return a pointer to array struct

2 Answers2