Не является ли этот устаревший код, который возвращает локальный массив символов, неправильным?

Question

Я наткнулся на какой-то устаревший код, который содержит такую ​​функцию:

LPCTSTR returnString()
{
    char buffer[50000];
    LPCTSTR t;

    /*Some code here that copies some string into buffer*/

    t = buffer;
    return t; 
}

Теперь я сильно подозреваю, что это неправильно. Я попытался вызвать функцию, и она возвращает строку, которую вы ожидаете вернуть. Однако на самом деле я не вижу, как это происходит: разве массив char не должен храниться в стеке и, следовательно, освобождаться после выхода из функции? Если я ошибаюсь, и она сохраняется в куче, разве эта функция не создает утечку памяти?

Answer 1

Ваш код демонстрирует неопределенное поведение - в этом случае UB - это то, что он "работает". Если вы хотите, чтобы массив хранился в куче, вам нужно выделить его с помощью new []. Затем вызывающая функция будет ответственна за удаление ее через указатель, который возвращает функция.

Answer 2

Нет утечки памяти, но функция по-прежнему неверна - buffer действительно создан в стеке, и тот факт, что вызывающая сторона может прочитать это, просто удача (она доступна только сразу после вызова returnString() function.) Этот буфер может быть перезаписан любыми дальнейшими вызовами функций или другими манипуляциями со стеком.

Правильный способ передачи данных по цепочке вызовов - предоставить буфер и размер для функции для заполнения.

Answer 3

Вы правы, это не гарантированно сработает; и на самом деле, если вы действительно храните строку длиной 50 000 символов в этом буфере, то после этого вызываете некоторую функцию (которая вызывает функцию, которая вызывает функцию ...), почти в каждой системе эта строка будет повреждена из-за кадр стека помещается в стек.

Единственная причина, по которой это работает, заключается в том, что устаревшая память стека (в большинстве систем) не очищается после возврата функции.

[Редактировать] Для пояснения, похоже, это работает, потому что у стека не было возможности увеличить 50 000 байтов. Попробуйте изменить его на char buffer[10]; и вызвать некоторые функции после returnString(), и вы увидите, что он поврежден.

Answer 4

Ваше чувство верно; код очень неправильный.

Память действительно находится в стеке и уходит с завершением функции.

Если бы буфер был статическим , вы могли бы по крайней мере ожидать, что он будет работать для одного вызова за раз (в однопоточном приложении).

Answer 5

Существует очень небольшая разница между массивом и указателем в C / C ++. Итак утверждение:

t = buffer;

На самом деле работает, потому что «буфер» означает адрес массива. Адрес явно не хранится в памяти, хотя, пока вы не поместите его в t (то есть буфер не является указателем). buffer [n] и t [n] будут ссылаться на один и тот же элемент массива. Ваш массив размещен в стеке, поэтому память освобождается, а не очищается, когда функция возвращается. Если вы посмотрите на него до того, как он будет перезаписан чем-то другим, то все будет хорошо.

Answer 6

Я согласен с тем, что больше всего здесь происходит неправильный возврат адреса автоматической переменной, однако я повторяю KevenK, что это не гарантируется, если это C ++, как указывает тег. Мы не знаем, что такое LPCTSTR. Что, если включенный заголовок содержит что-то вроде этого:

(да, я знаю, что это утечка, не в этом суть)

class LPCTSTR{
private:
  char * foo;

public:
  LPCTSTR & operator=(char * in){
    foo = strdup(in);
  }

  const char * getFoo(){
    return foo;
  }


};

Answer 7

Это опасная ошибка, скрывающаяся в вашем коде. В C и C ++ вам не разрешено вернуть указатель на данные стека в функции. Это приводит к неопределенному поведению. Я объясню почему.

Программа на C / C ++ работает, помещая данные в стек программ и из него. Когда вы вызываете функцию, все параметры помещаются в стек, а затем все локальные переменные также помещаются в стек. По мере выполнения программы она может помещать и выгружать больше элементов в стек вашей функции. В вашем примере, буфер помещается в стек, а затем t помещается в стек. Стек может выглядеть так:

• Предыдущий стек
• Параметры
• (другие данные)
• буфер (50000 байт)
• т (размер указателя)

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

Если вызывающая функция затем смотрит на то, на что t указывает , она обнаружит, что он указывает на память в стеке, которая может существовать или не существовать. (Время выполнения выскочило вне стека, но данные в стеке все еще могут быть там по стечению обстоятельств, а может и нет).

Хорошая новость в том, что она не безнадежна. Есть автоматизированные инструменты, которые могут искать такого рода ошибки в вашем программном обеспечении и сообщать о них. Они называются статическими инструменты анализа. Sentry является одним из таких примеров программы, которая может сообщить об этом вид дефекта.

Answer 8

Хотя все думают, что поведение не определено, и в этом случае оно кажется верным, в подобных случаях важно рассмотреть другие возможности.

Например, перегруженный operator=(const char*) может быть за кадром, выделяя память требований. Хотя это не так (насколько мне известно) с typedefs от Microsoft, но важно знать об этом в подобных случаях.

В этом случае, однако, кажется просто удобным, что он работает, и это определенно не гарантированное поведение. Как уже отмечали другие, это действительно должно быть исправлено.

Answer 9

Вы правы, код неправильный. :)

Вы должны изучить распределение памяти в C / C ++. Данные могут находиться в двух областях: стек и куча. Локальные переменные хранятся в стеке. malloc ed и new ed данные хранятся в куче. Состояние стека является локальным для функции - переменные живут в кадре стека - контейнер, который будет освобожден при возврате функции. Так что указатели ломаются.

Куча глобальная, поэтому все данные хранятся там до тех пор, пока программист явно не delet ed или free d. Вы можете положиться на эту область.

Answer 10

этот код возвращает указатель на память, выделенную в стеке. это очень опасно, потому что если вы попытаетесь передать этот указатель другой функции, память будет перезаписана вторым вызовом функции.

вместо этого вы можете использовать статический буфер:

static char buffer[50000];

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