2011-01-17 3 views
3

Рассмотрим следующий код:C++ ошибка - возвращение в массив символов

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 выдает ошибку, потому что функция возвращает массив. Что мне делать?

Мой друг сказал мне изменить подпись на void CeaserCrypt(char str[256], char encrypted[256], int key). Но я не думаю, что это правильно. Как я могу избавиться от ошибки компиляции?

+2

Интересно, сколько ответов и сколько обсуждений вызывает этот простой вопрос. –

+0

Это праздник в США, День MLK. – ThomasMcLeod

ответ

7

Тип возврата должен быть char *, но это только добавит еще одну проблему.

encrypted «выделено» в стеке и может не совпадать с действительностью при возврате функции. Поскольку encrypted будет иметь такую ​​же длину, как и вход, сделайте следующее:

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

encrypted[len] = '\0'; 

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

Не забудьте освободить буфер позже, хотя (с free()).

EDIT: @Yosy: не чувствуйте себя обязанным просто копировать/вставлять. Используйте это как указатель , чтобы улучшить свою практику кодирования. Кроме того, чтобы удовлетворить критику: передайте уже выделенный указатель на вашу процедуру шифрования, используя приведенный выше пример .

+5

Неплохая практика распределения буфера, который должен быть освобожден вызывающим. Вероятно, это приведет к утечкам памяти (поскольку никто не идеален). – ssmir

+1

Geez, люди. Вопрос не в том, «как правильно распределить/освободить память?». –

+0

@ssmir Правда, но, учитывая API, предоставленный OP, это самое простое решение, которое работает. Обязательно ли наша работа переписывать его API для поощрения другого стиля? ... <думать об этом> ОК, да, это, вероятно, не плохая идея поощрять лучший стиль, но это не то, о чем попросил ОП. –

-1

Хорошо, что вы возвращаете, это не символ, а массив символов. Попробуйте изменить тип возвращаемого на символ * (символ * и массив символов являются якобы то же самое для компилятора)

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

EDIT: как сказано в других постах, зашифрованный массив, вероятно, не будет действительным после функции вызов. вы всегда можете сделать новое объявление [] для шифрования, не забывая удалить его позже.

+1

-1 для возврата указателя на переменную, выделенную в стеке – ssmir

+0

Я бы поставил еще -1, если бы мог, для редактирования. Это ужасно плохо, чтобы вернуть указатель, который должен быть освобожден таким образом – ssmir

3

Поскольку вы используете C++, вы можете просто использовать std::string. Но в противном случае то, что предложил ваш друг, вероятно, лучше всего.

6

Он хочет, чтобы вы вернули символ char *, а не символ. Независимо от того, вы не должны возвращать ссылку или указатель на то, что вы создали в стеке. Вещи, выделенные в стеке, имеют a lifetime that corresponds with their scope. После окончания области действия эти переменные стека могут уходить.

Верните a std::vector вместо массива.

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; 
} 

Есть еще одна тонкая проблема: вы бросаете целое число в значение символа. Максимальный размер int намного больше, чем символ, поэтому ваш бросок может урезать значение.

+0

+1 для этого. Не используйте malloc и бесплатно, используйте RAII. – Puppy

+0

@DeadMG, где вы нашли RAII здесь? – Andrey

+4

@Andrey: В std :: vector? – Puppy

2

Если вам нужна совместимость с C, сделайте аргумент функции зашифрованной строки. Если нет, используйте C++ std :: string вместо строки стиля C.

А также В коде зашифрована строка не заканчивается «\ 0»

0

VS2010 является «кричащим» на вас, потому что вы пытаетесь вернуть значение, которое выделяется в стеке, и больше не действует после вызова функции.

У вас есть два варианта: 1) выделить память в куче внутри вашей функции или 2) использовать память, предоставленную вам вызывающим абонентом. Номер 2 - это то, что ваш друг предлагает и очень хороший способ сделать что-то.

Для 1 вам необходимо позвонить malloc() или new в зависимости от того, работаете ли вы на C или C++. В C, я бы следующее:

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

Для C++, если вы не хотите использовать строку, попробуйте

char* encrypted = new char[256]; 

Редактировать: FACEPALM Sorry о C шум, я должен был внимательно рассмотреть вопрос и понял, что вы работаете на C++.

1

Проблема с исходным кодом заключается в том, что вы пытаетесь вернуть указатель char* (к которому ваш локальный массив распался) от функции, прототипированной как возвращающая char. Функция не может возвращать массивы в C, а также в C++.

Возможно, ваш друг предположил, что вы меняете функцию таким образом, чтобы вызывающий абонент отвечал за выделение необходимого буфера.

Обратите внимание, что следующие прототипы полностью равным. Вы не можете передать массив как параметр нормальной функции.

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

OTOH, вы должны (если можете!) Решить, какой язык вы используете. Лучшая версия оригинала (в 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; 
} 

Обратите внимание, что переполнение знакового целого вызывает неопределенное поведение.

3

Здесь есть несколько проблем. Прежде всего:

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

Как указывали другие, неверный тип возврата. Вы не можете вернуть в один символ весь массив. Вы можете вернуть char*, но это возвращает указатель на массив, который будет выделен локально в стеке, и поэтому будет недействительным после удаления кадра стека (после функции в основном). В английском языке, вы будете получать доступ к этому адресу памяти, но кто знает, что там будет ...

Как ваш друг предложил, лучше подпись будет:

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

Я добавил несколько things - a size_t length, чтобы вы могли обрабатывать любую длину. Таким образом, размер str может быть определен по мере необходимости. Просто убедитесь, что char* encrypted имеет тот же размер.

Тогда вы можете сделать:

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

Для этого, чтобы работать ваш абонент будет необходимо распределили соответствующим образом размера буфера одинаковой длины, длина которой вы должны пройти в в параметре длины. Посмотрите malloc для C. Если C++, используйте std::string.

+0

+1 для первой части ответа (исключая часть о переполнении) – ssmir

+0

Я написал часть о переполнениях и теперь не могу понять себя, если я прав или нет ... это не по теме, но я все еще хочу быть уверенным. –

+0

Наполните его, удалите его, пока я не знаю, что я имею в виду (здесь уже поздно). –

0

Вы можете просто сделать свой шифр Ceaser на месте, нет необходимости передавать и выходить массивы.

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

В качестве дальнейшего упрощения пропустите возвращаемое значение.

void CeaserCrypt(char str[256], int key) 
{  
    for(unsigned i = 0; i < strlen(str); i++)  
    { 
     str[i] += key;  
    } 
} 
Смежные вопросы