String returned from function contains garbage

Question

I want the function lch() to return a string that can be used outside the function.
This is the code I have written, but it does not seem to work:

char *lch(char *ch,int n){ 
    char c[n];
    for(int i=0;i<n;i++){
            c[i] = *ch;
    }
    puts(c); // check output string inside function 
    return c;
}

char str[100],*p;

main(){
    p = lch("a",20);
    puts(p); // check output outside function
}

I am confused with strings and how they should be passed to functions.
I want the output string to become the same on both calls to puts().
What should I do?

That is the result of the code above:

aaaaaaaaaaaaaaaaaaaa               // inside the function 
                      ¢ÞêÊ·      // outside the function

You have several problems here, all of them are duplicates of many existing questions. You might want to research why it is a bad idea to 1) return a locally defined array, 2) Not to terminate your string with `'\0'` character. — Eugene Sh., Sep 05 '19 at 19:41
Note that this isn't closed as a duplicate to discourage you; the duplicate link is meant to answer your question. If the answers on the duplicate question do not answer your question, edit your question and it may be reopened. — S.S. Anne, Sep 05 '19 at 20:48

S.S. Anne · Accepted Answer · 2019-09-05T20:40:48.570

First of all, returning locally allocated storage will not work. You have to return dynamically allocated storage, and with a proper size to accommodate the null-terminator:

char *c = malloc(n+1);
/* ... */
/* end of program: */
free(p);

Second, you want to pass a character to your function, not a string:

char *lch(char ch,int n){
/* ... */
    c[i] = ch;
/* ... */
p = lch('a', 20);

Third, you have to null-terminate your string:

int i;
for(i=0;i<n;i++){
        c[i] = ch;
}
ch[i] = '\0';
puts(c); //check output string inside function

Here's the dynamically-allocated storage approach:

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

char *lch(char ch,int n){ 
    char *c = malloc(n+1);
    int i;
    for(i=0;i<n;i++){
            c[i] = ch;
    }
    c[i] = '\0';
    puts(c); //check output string inside function 
    return c;
}

char *p;

int main(void){
    p = lch('a',20);
    puts(p); //check output outside function
    free(p);
    return 0;
}

This also fixes the declaration and return type of main (main is supposed to be int main(void) or int main(int argc, char **argv)), removes the unneeded variable str, and adds needed #includes.

Tal Avraham · Answer 2 · 2019-09-05T20:42:28.803

The reason your code does not do what you expect it to do is because the string in the function is allocated on the stack, which means that its memory is cleaned as soon as you exit the function.

That means the pointer p points to garbage value after the call to lch().

Also, you can not declare an array of size that is not a constant value, so the line:

char c[n];

would simply not work.

To solve this, you will need to dynamically allocate the string using malloc(3) :

char* lch(char ch, int n)
{ 
    char * c;

    /* Allocate n bytes of memory for the string */
    c = malloc(n + 1);
    if (NULL == c)
    {
        /* Failed to allocate memory, exit the function */
        return c;
    }

    for(int i = 0; i < n; i++)
    {
        c[i] = ch;
    }
    /* Add a terminating null byte (to make it a string) */
    c[i] = '\0'; 

    puts(c);
    return c;
}

int main(void)
{
    char * p;
    p = lch('a', 20);
    puts(p);

    /* Free the string from the memory */
    free(p);

    return 0;
}

I added a few fixes to the code but the main thing you need to look at is the use of malloc(3).

I dynamically allocated n+1 bytes of memory for the string, then wrote the data into the string (plus a '\0'), and when the function exits the memory will still be available and wont be corrupted. The call to free is needed to free the memory we have allocated.

You can read more about malloc(3) here: https://linux.die.net/man/3/malloc

`char c[n];` does work in C11 (I think?) and later. It's called a "variable length array", or VLA for short.. — S.S. Anne, Sep 05 '19 at 20:45

String returned from function contains garbage

2 Answers2