CS50: error: address of stack memory associated with local variable-CodePudding

I've tried different ways to assign a string "cypher" using function "cypher_text()". I pass two strings to funtion cypher_text(), and try to pass a string back. I'm not sure if I'm trying to pass a string or an array of chars back. But I thought they were essentially the same, as in a string is an array of chars.

I can complete the assignment by just printing the values in the function - but I want to understand why I can't return the string from the cypher_text() function back to the main().

#include <stdio.h>
#include <cs50.h>
#include <string.h>
#include <ctype.h>

string cypher_text(string text, string key);

int main(int argc, string argv[])
{
    string cypher;

    if (argc != 2)
    {
        printf("Missing Key\n");
        return 1;
    }
    if (strlen(argv[1]) != 26)
    {
        printf("Invalid Key - must be 26\n");
        return 1;
    }

    for (int i=0,n=strlen(argv[1]); i<n; i  )
    {
        if (isalpha(argv[1][i])==0)
        {
            printf("Invalid key must have only letters\n");
            return 1;
        }
        //for char[0] check if equals to char[1], char[2]....
        for (int j=i 1,m=strlen(argv[1]); j<m; j  )
        {
            if (argv[1][i] == argv[1][j])
            {
                printf("Invalid key duplicates\n");
                return 1;
            }
        }
    }
    
    string plain = get_string("Plain text:");
    
    printf("cypher text: ");
    cypher = cypher_text(plain, argv[1]);
    printf("\n");
    
    //printf("Cypher text:%s\n",cypher);

}


string cypher_text(string text, string key)
{
    int value;
    char cypher[strlen(text)];
    for (int i=0, n=strlen(text);i<n;i  )
    {
        if (isalpha(text[i]))
        {
            if (isupper(text[i]))
            {
                value = (int)text[i]-65;
                cypher[i] = key[value];
                printf("%c",key[value]);
            }
            else
            {
                value = (int)text[i]-97;
                //cypher[i] = key[value];
                printf("%c",(int)key[value] 32);
            }
        }
        else
        {
            //cypher[i] = text[i];
            printf("%c",text[i]);
        }
    }
    
    return cypher;
}

CodePudding user response：

You have two problems with the cypher_text function.

The first is that

char cypher[strlen(text)];

does not make room for the string null-terminator. You need to allocate space for the terminator, as well as adding it to yhr buffer.

The second problem is that it's a local variable, whose life-time ends when the function ends. So the pointer you return from the function will immediately be invalid.

Two solutions to the second problem:

Pass a pointer to an allocated buffer of the needed length (including the null-terminator), and use it instead;
Or allocate memory dynamically, which you return a pointer to. This requires you to free the memory once you're done with it otherwise you will have a memory leak.

CodePudding user response：

I want to understand why I can't return the string from the cypher_text() function back to the main().

Short answer - you cannot return an array from a function in C. What actually gets returned is a pointer to the first element of the array; unfortunately, once the function exits, that array ceases to exist and that pointer is no longer valid.

C's treatment of arrays is different from most languages'. When the compiler sees an expression of type "N-element array of T", that expression will be converted (or "decay") to an expression of type "pointer to T" and the value of the expression will be the address of the first element unless:

the array expression is the operand of the sizeof, _Alignof, or unary & operators; or
the expression is a string literal being used to initialize a character array in a declaration.

There is a reason for this - it's not an arbitrary thing just to drive people crazy. It allows C to mimic the same array subscripting behavior as its precursor language B. Unfortunately, it means arrays lose their "array-ness" under most circumstances and what you're actually working with is a pointer. When you write

return cypher;

in your function, what the compiler sees is

return &cypher[0];

The string type in the CS50 library is a lie; it's just a typedef (alias) for the type char * (pointer to char). C doesn't have an actual string datatype as such. In C, a string is just a sequence of character values including a 0-valued terminator. Strings (including string literals like "hello") are stored in arrays of character type (char for "normal" character encodings like ASCII, EBCDIC, and UTF-8, and wchar_t for "wide" encodings like UTF-16). But character arrays can also store sequences that aren't strings.

So, when you need to create a new string and return it to the caller, you have three options:

Have the caller pass the target array (along with its size!!!) as a parameter to the function; again, what the function receives is a pointer to the first element:

void cypher_text(string text, string key, string cypher, size_t cypher_size) // or char * cypher, since that's what's really happening
{
  ...
  cypher[i] = ...;
  ...
}

int main( void )
{
  ...
  char cypher[TEXT_LENGTH 1]; //  1 for 0 terminator
  ...
  cypher_text( plain, argv[i], cypher, sizeof cypher );
  ...
}

This is my preferred option, as the caller is responsible for managing the memory of the cypher array. Life's just simpler if one entity keeps track of the lifetime.

Alternately, you could have cypher_text dynamically allocate the buffer to store the cypher text - dynamically allocated memory stays allocated until you explicitly free it:

string cypher_text( string plain, string key )
{
  string cypher = calloc( strlen( text   1 ), sizeof *cypher );
  if ( cypher )
  {
    // compute cypher and assign text here
  }
  return cypher;
}

This is what the CS50 get_string() function is doing for you behind the scenes - it's using calloc (or malloc, not sure at the moment) to allocate the memory in which the string is stored. Just make sure to free the array when you're done with it. Note that you could combine this approach with the first method - have main use calloc to allocate the memory for cypher and pass that pointer to cypher_text() - that way you don't have to declare cypher as a fixed-size array as I did in the first example.

Or, you could use a global array. I do not recommend this and will not provide an example because it's a bad idea, but I'm just including it for completeness. Sometimes using a global isn't the wrong answer, but this isn't one of those times.

Welcome to C. It's ... not intuitive. The CS50 library tries to hide the nastier parts of the language from you, but in the process grossly misrepresents how it actually works, and I think that doesn't do you any favors.