C++ error - returning a char array

Consider the following code:

char CeaserCrypt(char str[256],int key)
{
    char encrypted[256],encryptedChar;
    int currentAsci;

    encrypted[0] = '\0'; 

    for(int i = 0; i < strlen(str); i++) 
    {
        currentAsci = (int)str[i];
        encryptedChar = (char)(currentAsci+key);
        encrypted[i] = encryptedChar;
    }

    return encrypted;
}

Visual Studio 2010 gives an error because the function returns an array. What should I do?

My friend told me to change the signature to void CeaserCrypt(char str[256], char encrypted[256], int key). But I don't think that is correct. Ho开发者_开发百科w can I get rid of the compile error?

The return type should be char * but this'll only add another problem.

encrypted is "allocated" on the stack of CeaserCrypt and might not be valid when the function returns. Since encrypted would have the same length as the input, do:

int len = strlen(str);
char *encrypted = (char *) malloc(len+1);

encrypted[len] = '\0';

for (int i = 0; i < len; i++) {
// ...
}

Don't forget to deallocate the buffer later, though (with free()).

EDIT: @Yosy: don't feel obliged to just copy/paste. Use this as a pointer to improve your coding practice. Also, to satisfy criticizers: pass an already allocated pointer to your encryption routine using the above example.

It wants you to return a char* rather than a char. Regardless, you shouldn't be returning a reference or a pointer to something you've created on the stack. Things allocated on the stack have a lifetime that corresponds with their scope. After the scope ends, those stack variables are allowed to go away.

Return a std::vector instead of an array.

std::vector<char> CeaserCrypt(char str[256],int key)
{
    std::vector<char> encrypted(256);
    char encryptedChar;
    int currentAsci;

    encrypted[0] = '\0'; 

    for(int i = 0; i < strlen(str); ++i) 
    {
        currentAsci = (int)str[i];
        encryptedChar = (char)(currentAsci+key);
        encrypted[i] = encryptedChar;
    }

    return encrypted;
}

There's another subtle problem there though: you're casting an integer to a character value. The max size of an int is much larger than a char, so your cast may truncate the value.

Since you're using C++ you could just use an std::string instead. But otherwise, what your friend suggested is probably best.

There are a few problems here. First up:

char CeaserCrypt(char str[256],int key)

As others have pointed out, your return type is incorrect. You cannot return in a single character an entire array. You could return char* but this returns a pointer to an array which will be allocated locally on the stack, and so be invalid once the stack frame is removed (after the function, basically). In English, you'll be accessing that memory address but who knows what's going to be there...

As your friend suggested, a better signature would be:

void CeaserCrypt(char* encrypted, const char str*, const size_t length ,int key)

I've added a few things - a size_t length so you can process any length string. This way, the size of str can be defined as needed. Just make sure char* encrypted is of the same size.

Then you can do:

for(int i = 0; i < length; i++) 
{
    // ...

For this to work your caller is going to need to have allocated appropriately-sized buffers of the same length, whose length you must pass in in the length parameter. Look up malloc for C. If C++, use a std::string.

If you need C compatibility make encrypted string function argument. If not, than use C++ std::string instead C style string.

And also In your code encrypted string isn't ending with '\0'

The problem with the original code is that you are trying to return a char* pointer (to which your local array decayed) from a function that is prototyped as one returning a char. A function cannot return arrays in C, nor in C++.

Your friend probably suggested that you change the function in such a way, that the caller is responsible for allocation the required buffer.

Do note, that the following prototypes are completely equal. You can't pass an array as a parameter to normal function.

int func(char array[256]);
int func(char* array);

OTOH, you should (if you can!) decide the language which you use. Better version of the original (in C++).

std::vector<unsigned char> CeaserCrypt(const std::string& str, const int key)
{
    std::vector<unsigned char> encrypted(str.begin(), str.end());
    for (std::vector<unsigned char>::iterator iter = vec.begin();
         iter != vec.end(); ++iter) {
        *iter += key;
    }
    return vec;
}

Do note that overflowing a signed integer causes undefined behavior.

VS2010 is "yelling" at you because you are trying to return a value that is allocated on the stack, and is no longer valid once your function call returns.

You have two choices: 1) Allocate memory on the heap inside your function, or 2) use memory provided to you by the caller. Number 2 is what your friend in suggesting and is a very good way to do things.

For 1, you need to call malloc() or new depending on whether you are working in C or C++. In C, I'd have the following:

char* encrypted = malloc(256 * sizeof(char));

For C++, if you don't want to use a string, try

char* encrypted = new char[256];

Edit: facepalm Sorry about the C noise, I should have looked at the question more closely and realized you are working in C++.

You can just do your Ceaser cipher in place, no need to pass arrays in and out.

char * CeaserCrypt(char str[256], int key) 
{     
    for(unsigned i = 0; i < strlen(str); i++)      
    {
        str[i] += key;     
    }
    return str; 
} 

As a further simplification, skip the return value.

void CeaserCrypt(char str[256], int key) 
{     
    for(unsigned i = 0; i < strlen(str); i++)      
    {
        str[i] += key;     
    }
} 

well what you're returning isn't a char, but a char array. Try changing the return type to char*(char* and a char array are ostensibly the same thing for the compiler)

char* CeaserCrypt(char str[256],int key)

EDIT: as said in other posts, the encrypted array will probably not be valid after the function call. you could always do a new[] declaration for encrypted, remembering to delete[] it later on.