Dynamically allocated array won't print outside the function it's declared

Question

This program will scan a PGM file and store it's values to an array dynamically allocated img inside the function LerPGM(), the function will then return &img. As I declared PGM *imgconvI assign LerPGMto it (imgconv=LerPGM()). The thing is, the printfinside the function (printf("%d ", img.imagem[i][j]) works perfectly, but the printf("%d ", imgconv->imagem[i][j]) inside the main() only prints the first item and then the program stops working.

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

typedef struct{
    int c;
    int l;
    unsigned char maximo;
    unsigned char **imagem;
} PGM;

PGM *LerPGM(char* entrada);

int main()
{
    PGM *imgconv;
    int i, j;

    imgconv=LerPGM("entrada.pgm");
    for(i=0; i<imgconv->l; i++){
        for(j=0; j<imgconv->c; j++){
            printf("%d ", imgconv->imagem[i][j]);
        }
        printf("\n");
    }

    return 0;
}

PGM *LerPGM(char* entrada){
    PGM img;
    char tipo[3];
    int i, j;

    FILE *arq;
    arq = fopen(entrada, "r");
    if(arq == NULL){
        printf("Arquivo nao encontrado.");
        return 0;
    }

    fscanf(arq, "%s %d %d %d", &tipo, &img.c, &img.l, &img.maximo);
    if(strcmp(tipo, "P2")){
        printf("O arquivo nao e PGM.");
        return 0;
    }

    img.imagem = malloc(img.l * sizeof(char *));
    for(i=0; i<img.c; i++) img.imagem[i] = malloc(img.c * sizeof(char));
    if(img.imagem == NULL){
        printf("Falha na alocacao de memoria.");
        return 0;
    }

    for(i=0; i<img.l; i++){
        for(j=0; j<img.c; j++){
            fscanf(arq, "%d", &img.imagem[i][j]);
        }
    }

    fclose(arq);

    for(i=0; i<img.l; i++){
        for(j=0; j<img.c; j++){
            printf("%d ", img.imagem[i][j]);
        }
        printf("\n");
    }

    return &img;
}

It's been a long time since encountering this instance of undefined behavior on SO... — cadaniluk, Oct 06 '15 at 18:24
Why do you think `img` is dynamically allocated? It's a local variable. Dynamic allocation is done using `malloc()`. — Barmar, Oct 06 '15 at 18:45

mksteve · Accepted Answer · 2015-10-06T18:27:10.730

You are returning a pointer from a function, but the data was on the stack. That is why it doesn't work.

  X * func() {
      X data;

      return &data; /* <<== this is broken. */

  }

When func in my case, or LerPGM return, the data from the stack in the function is destroyed.

Fix - something like ...

PGM *LerPGM(char* entrada){
    PGM * img = (PGM*)malloc( sizeof( PGM ) );
    char tipo[3];
    int i, j;

    FILE *arq;
    arq = fopen(entrada, "r");
    if(arq == NULL){
        printf("Arquivo nao encontrado.");
        return 0;
    }

    fscanf(arq, "%s %d %d %d", &tipo, &img->c, &img->l, &img->maximo);
    if(strcmp(tipo, "P2")){
        printf("O arquivo nao e PGM.");
        return 0;
    }

    img->imagem = malloc(img->l * sizeof(char *));
    for(i=0; i<img->c; i++) img->imagem[i] = malloc(img->c * sizeof(char));
    if(img->imagem == NULL){
        printf("Falha na alocacao de memoria.");
        return 0;
    }

    for(i=0; i<img->l; i++){
        for(j=0; j<img->c; j++){
            fscanf(arq, "%d", &img->imagem[i][j]);
        }
    }

    fclose(arq);

    for(i=0; i<img->l; i++){
        for(j=0; j<img->c; j++){
            printf("%d ", img->imagem[i][j]);
        }
        printf("\n");
    }

    return img;
}

I changed it to http://pastebin.com/cvxya2yW and it seems to work now, thank you. — Eduardo Moura Tavares, Oct 06 '15 at 18:32

score 0 · Answer 2 · answered Oct 06 '15 at 20:13

You're returning a pointer to a local variable. That is a pointer to a variable, which gets destroyed and disappears on the return. After the return the place pointed may contain some other data, unrelated to the previous content of the img variable.

If you want to return data from the function, you should either prepare the variable outside the function and pass it to the function for filling:

int LerPGM(char* entrada, PGM *img){
    // ....

    FILE *arq = fopen(entrada, "r");
    if(arq == NULL){
        printf("Arquivo nao encontrado.");
        return 0;  // ERROR
    }

    // ....
    return 1;    // SUCCESS
}

int main()
{
    PGM imgconv;
    int i, j;

    if( LerPGM("entrada.pgm", &imgconv) ) {
        for(i=0; i<imgconv->l; i++){
            for(j=0; j<imgconv->c; j++){
                printf("%d ", imgconv->imagem[i][j]);
            }
            printf("\n");
        }
    }
    else
        printf("No data to process.\n");

    return 0;
}

or allocate a new variable on a heap inside the function and return a pointer to it (then remember to release the variable after use):

PGM *LerPGM(char* entrada){
    PGM *img = NULL;
    // ....

    FILE *arq = fopen(entrada, "r");
    if(arq == NULL){
        printf("Arquivo nao encontrado.");
        return NULL;  // ERROR
    }

    // ....
    img = malloc(sizeof(PGM));
    if(img != NULL)
    {
        // fill variable *img with data
    }

    return img;
}

int main()
{
    PGM* imgconv;
    int i, j;

    // ...
    imgconv = LerPGM("entrada.pgm");
    if(imgconv != NULL) {
        for(i=0; i<imgconv->l; i++){
            for(j=0; j<imgconv->c; j++){
                printf("%d ", imgconv->imagem[i][j]);
            }
            printf("\n");
        }
        free(imgconv->subcomponents);
        free(imgconv);
    }
    else
        printf("No data to process.\n");

    return 0;
}

Dynamically allocated array won't print outside the function it's declared

2 Answers2