2

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.

calderonmluis
  • 547
  • 4
  • 11
  • How do you initialize `input_string`? Do you allocate memory for it? What does "prints nothing" mean? – JohnB Sep 14 '12 at 03:18
  • Have you compiled with warnings? I.e. (with gcc) $ gcc -Wall -Wextra -pedantic -std=c89 ... – Morpfh Sep 14 '12 at 03:19
  • I instantiate input_string like this: char* input_ptr; char input_string[LINE_LEN]; I then add some stuff to it and pass the input_ptr to func(). – calderonmluis Sep 14 '12 at 03:23
  • @Morpfh I compiled with warnings and get none. – calderonmluis Sep 14 '12 at 03:25
  • The next step then (in such a case) could be to compile with -ggdb flag and run with valgrind: $ valgrind ./my_prog – Morpfh Sep 14 '12 at 03:27
  • @Morpfh I am on OSX and do not have valgrind. Would there be another way? If necessary I can try it on a linux box. – calderonmluis Sep 14 '12 at 03:29
  • @calderonmluis Do you set `input_ptr = input_string`? If you never assign anything to `input_ptr`, then it is uninitialized. A pointer is just a number that gives a location in memory. You need to actually give it a value. – paddy Sep 14 '12 at 03:38
  • @calderonmluis: MallocDebug perhaps: http://stackoverflow.com/questions/7490825/valgrind-like-tool-on-mac-os-10-7-lion – Morpfh Sep 14 '12 at 03:45
  • @paddy I do initialize the -input_ptr-. I updated the code above to show more – calderonmluis Sep 14 '12 at 03:49
  • Why don't you have `valgrind` on Mac OS X? It compiles and runs fine for me (Mac OS X 10.7.4; GCC 4.7.1). – Jonathan Leffler Sep 14 '12 at 03:49
  • @JonathanLeffler Thanks for the tip I will begin installing it now. – calderonmluis Sep 14 '12 at 03:50

2 Answers2

2

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;
}
paddy
  • 60,864
  • 6
  • 61
  • 103
2

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.

Community
  • 1
  • 1
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Thanks for the tip I will definitely use these flags during compilation from now on. – calderonmluis Sep 14 '12 at 03:58
  • Using gcc and compiling with -Wall -Wextra -pedantic does not give any warning on this. The reason being he returns input_ptr and not &input_string[0]; – A bit tricky that one. – Morpfh Sep 14 '12 at 04:07