CODEWITHSUNDEEP
cmallocfreeundefined-behavior
calderonmluis
calderonmluis

Reputation: 547

Dynamic Array malloc empties separate array

I am having an issue with dynamic arrays and malloc. I am fairly new to C so please excuse (and advise on) any rookie mistakes.

The problem is I create an array (input_string in this case) and pass it to func2. Then in func2 I do a test, print out the first element of input_string.

This works as expected in the first printout before the malloc, but after the malloc it doesn't print anything. This seems weird to me since in between the to printf statements I do nothing to the input_string.

I'm assuming that I am dealing with these arrays incorrectly, but I'm unsure.

Here is a snippet of the code in question:

Updated

... // includes not in snippet

/* CONSTANTS */
#define LINE_LEN 80

/* Function declarations */
char* func1(void);
char* func2(int tl, char* input_string);

int main(void) {
    char* input_string;
    int tab_length;
    char* output_string;

    input_string = func1();
    output_string = func2(tl, input_string);

    return 0;
}

char* func1(void) {
    char cur_char;
    char* input_ptr;
    char input_string[LINE_LEN];
    while ((cur_char = getchar()) != '\n' && chars_read < 80) {
        // iterate and create the array here
    }
    input_ptr = &input_string[0]; /* set pointer to address of 0th index */
    return input_ptr;
}

char* func2(int tl, char* input_string) {
    int n = 0, output_idx = 0;
    char* output_ptr;
    printf("\nBefore malloc: %c ", *(input_string));
    output_ptr = malloc(tab_length * chars_read+1);
    if (output_ptr == NULL) {
            printf("Failed to allocate memory for output_ptr.\nExiting");
            exit(1);
    }
    printf("\nAfter malloc: %c ", *(input_string));
    ...
    return output_ptr;
}

P.s.: Any undeclared variables have been declared outside of this snippet of code.

Update

Thanks for all the replies and advice. It is very much appreciated.

Upvotes: 2

Views: 143

Answers (2)

Jonathan Leffler
Jonathan Leffler

Reputation: 755094

One primary problem is that you return a pointer to a local array in:

char* func1(void)
{
    char cur_char;
    char* input_ptr;
    char input_string[LINE_LEN];
    while ((cur_char = getchar()) != '\n' && chars_read < 80) {
        // iterate and create the array here
    }
    input_ptr = &input_string[0]; /* set pointer to address of 0th index */
    return input_ptr;
}

You've set input_ptr to point to the local array input_string (the zeroth element of it), and then return that pointer. Once the function returns, the pointer is invalid; the space is reused for other purposes.

You should be compiling with more warnings set if you didn't get a compiler warning anyway (and you should not be ignoring warnings until you know enough C to know why you can safely ignore the warning). I use -Wall -Wextra almost always, and -Wall essentially always.

As Morpfh points out, GCC (4.7.1 tested on Mac OS X 10.7.4) does not warn about returning the pointer, so you will have to be aware of that without assistance from the compiler. (If you change the return to return input_string; or return &input_string[0];, then the compiler does give you a useful warning.)

Also, while converting the code into a compilable unit, I notice that you do not take care of EOF, and you assign the result of getchar() to a char; you shouldn't because it returns an int (see 'fgetc() checking EOF' amongst many other questions for another discussion of this issue). You also need to ensure your string is null terminated. You should also avoid repeating constants, so rather than using 80 in the condition, use LINE_LEN (or, even better, sizeof(input_string)-1). And you have to watch out for off-by-one buffer overflows. So, my compilable version of your func1() was:

#include <stdio.h>
#include <stdlib.h>
#define LINE_LEN 80
extern char *func1(void);
char *func1(void)
{
    int cur_char;
    char *input_ptr;
    int chars_read = 0;
    char input_string[LINE_LEN+1];

    while ((cur_char = getchar()) != EOF && cur_char != '\n' && chars_read < LINE_LEN)
        input_string[chars_read++] = cur_char;

    input_string[chars_read] = '\0';
    input_ptr = &input_string[0]; /* set pointer to address of 0th index */
    return input_ptr;
}

This is still broken code because it returns the local pointer.

Upvotes: 2

paddy
paddy

Reputation: 63501

func1 Returns a pointer to a temporary string. You did not allocate it. This will produce undefined behaviour.

Instead you should do this:

char* func1(void) {
    char cur_char;
    char* input_ptr = (char*)malloc(LINE_LEN * sizeof(char));
    while ((cur_char = getchar()) != '\n' && chars_read < 80) {
        // iterate and create the array here
    }
    return input_ptr;
}

Interestingly, you did use malloc inside func2.

When you are finished with it, you need to call free to release the memory.

int main(void) {
    char* input_string;
    int tab_length;
    char* output_string;

    input_string = func1();
    output_string = func2(tl, input_string);

    free(input_string);
    free(output_string);    

    return 0;
}

Upvotes: 2

Related Questions