Почему я получаю двойную бесплатную ошибку с realloc ()? - PullRequest
12 голосов
/ 04 августа 2008

Я пытался написать функцию замены строки в C, которая работает на char *, который был выделен с использованием malloc(). Он немного отличается тем, что будет искать и заменять строки, а не символы в начальной строке.

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

Возможно, небольшой код поможет:

void strrep(char *input, char *search, char *replace) {
    int searchLen = strlen(search);
    int replaceLen = strlen(replace);
    int delta = replaceLen - searchLen;
    char *find = input;

    while (find = strstr(find, search)) {

        if (delta > 0) {
            realloc(input, strlen(input) + delta);
            find = strstr(input, search);            
        }

        memmove(find + replaceLen, find + searchLen, strlen(input) - (find - input));
        memmove(find, replace, replaceLen);
    }
}

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

Если это помогает, код вызова выглядит следующим образом:

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

void strrep(char *input, char *search, char *replace);

int main(void) {
    char *input = malloc(81);

    while ((fgets(input, 81, stdin)) != NULL) {
        strrep(input, "Noel", "Christmas");
    }
}

Ответы [ 8 ]

12 голосов
/ 04 августа 2008

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

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

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

В результате:

void  strrep(char *input, char *search, char *replace);
char* strrepm(char *input, char *search, char *replace);
void  strrepmfree(char *input);
11 голосов
/ 09 августа 2008

Прежде всего, извините, я опоздал на вечеринку. Это мой первый ответ на stackoverflow. :)

Как уже указывалось, при вызове realloc () вы можете потенциально изменить указатель на перераспределяемую память. Когда это происходит, аргумент «строка» становится недействительным. Даже если вы переназначите его, изменение выйдет из области действия после завершения функции.

Чтобы ответить на OP, realloc () возвращает указатель на вновь перераспределенную память. Возвращаемое значение нужно где-то хранить. Как правило, вы бы сделали это:

data *foo = malloc(SIZE * sizeof(data));
data *bar = realloc(foo, NEWSIZE * sizeof(data));

/* Test bar for safety before blowing away foo */
if (bar != NULL)
{
   foo = bar;
   bar = NULL;
}
else
{
   fprintf(stderr, "Crap. Memory error.\n");
   free(foo);
   exit(-1);
}

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

void foobar(char *input, int newlength)
{
   /* Here, I ignore my own advice to save space. Check your return values! */
   input = realloc(input, newlength * sizeof(char));
}

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

Вы можете использовать двойной указатель для ввода, например:

void foobar(char **input, int newlength)
{
   *input = realloc(*input, newlength * sizeof(char));
}

Если у вызывающей стороны есть дубликат указателя ввода где-то, этот дубликат все еще может быть недействительным.

Я думаю, что самое чистое решение здесь - избегать использования realloc () при попытке изменить ввод функции вызывающей стороны. Просто возьмите malloc () новый буфер, верните его, и пусть вызывающая сторона решит, освобождать или нет старый текст. Это дает дополнительное преимущество, позволяя вызывающей стороне сохранять исходную строку!

6 голосов
/ 21 октября 2008

Кто-то еще извинился за опоздание на вечеринку - два с половиной месяца назад. О, хорошо, я провожу довольно много времени, занимаясь археологией программного обеспечения.

Меня интересует, что никто не прокомментировал явно утечку памяти в оригинальном дизайне или ошибку «один за другим». И именно наблюдение за утечкой памяти говорит мне точно, почему вы получаете ошибку двойного освобождения (потому что, если быть точным, вы освобождаете одну и ту же память несколько раз - и вы делаете это после того, как попираете уже освобожденную память).

Перед проведением анализа я согласен с теми, кто говорит, что ваш интерфейс менее звездный; однако, если вы столкнулись с проблемами утечки / утечки памяти и задокументировали требование «должна быть выделена память», это может быть «ОК».

В чем проблемы? Ну, вы передаете буфер функции realloc (), и realloc () возвращает вам новый указатель на область, которую вы должны использовать - и вы игнорируете это возвращаемое значение. Следовательно, realloc (), вероятно, освободил исходную память, а затем вы снова передаете ему тот же указатель, и он жалуется, что вы освобождаете одну и ту же память дважды, потому что вы снова передаете ей исходное значение. Это не только приводит к утечке памяти, но и означает, что вы продолжаете использовать исходное пространство - и выстрел Джона Дауни в темноте указывает на то, что вы неправильно используете realloc (), но не подчеркивает, насколько серьезно вы это делаете. Также есть ошибка «один за другим», потому что вы не выделяете достаточно места для NUL '\ 0', заканчивающего строку.

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

Ваш код также не защищает от неопределенного роста - рассмотрите возможность замены 'Noel' на 'Joyeux Noel'. Каждый раз вы добавляете 7 символов, но вы найдете другой Ноэль в замененном тексте, расширяете его и так далее, и так далее. Мое исправление (ниже) не решает эту проблему - простое решение, вероятно, состоит в том, чтобы проверить, появляется ли строка поиска в строке замены; альтернатива - пропустить строку замены и продолжить поиск после нее. Второй имеет несколько нетривиальных проблем с кодированием.

Итак, моя рекомендуемая версия вызываемой функции:

char *strrep(char *input, char *search, char *replace) {
    int searchLen = strlen(search);
    int replaceLen = strlen(replace);
    int delta = replaceLen - searchLen;
    char *find = input;

    while ((find = strstr(find, search)) != 0) {
        if (delta > 0) {
            input = realloc(input, strlen(input) + delta + 1);
            find = strstr(input, search);            
        }

        memmove(find + replaceLen, find + searchLen, strlen(input) + 1 - (find - input));
        memmove(find, replace, replaceLen);
    }

    return(input);
}

Этот код не обнаруживает ошибки выделения памяти - и, вероятно, аварийно завершает работу (но если нет, то приводит к утечке памяти) в случае сбоя функции realloc (). См. Книгу Стива Магуайра «Написание твердого кода» для подробного обсуждения вопросов управления памятью.

6 голосов
/ 04 августа 2008

Просто выстрел в темноте, потому что я еще не пробовал, но когда вы перераспределяете, он возвращает указатель, очень похожий на malloc. Поскольку realloc может перемещать указатель при необходимости, вы, скорее всего, работаете с недопустимым указателем, если не выполните следующие действия:

input = realloc(input, strlen(input) + delta);
4 голосов
/ 04 августа 2008

Обратите внимание, попробуйте отредактировать код, чтобы избавиться от escape-кодов html.

Ну, хотя с тех пор, как я использовал C / C ++, прошло некоторое время, но растущий realloc использует значение указателя памяти только в том случае, если после исходного блока в памяти есть место.

Например, рассмотрим это:

(XXXXXXXXXX ..........)

Если ваш указатель указывает на первый х, и. означает свободное место в памяти, и вы увеличиваете объем памяти, на который указывает ваша переменная, на 5 байт, это будет успешно. Это, конечно, упрощенный пример, поскольку блоки округляются до определенного размера для выравнивания, но в любом случае.

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

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

Однако это значение указателя было освобождено.

В вашем случае вход является виновником.

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

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

3 голосов
/ 17 мая 2011

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

Я видел код, где

realloc(bytes, smallerSize);

использовался и работал, чтобы изменить размер буфера, сделав его меньше. Сработало около миллиона раз, а затем по какой-то причине realloc решила, что даже если вы укоротите буфер, он даст вам хорошую новую копию. Таким образом, вы попадаете в случайное место через полсекунды после того, как случилось плохое.

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

3 голосов
/ 04 августа 2008

Это похоже на работу;

char *strrep(char *string, const char *search, const char *replace) {
    char *p = strstr(string, search);

    if (p) {
        int occurrence = p - string;
        int stringlength = strlen(string);
        int searchlength = strlen(search);
        int replacelength = strlen(replace);

        if (replacelength > searchlength) {
            string = (char *) realloc(string, strlen(string) 
                + replacelength - searchlength + 1);
        }

        if (replacelength != searchlength) {
            memmove(string + occurrence + replacelength, 
                        string + occurrence + searchlength, 
                        stringlength - occurrence - searchlength + 1);
        }

        strncpy(string + occurrence, replace, replacelength);
    }

    return string;
}

Вздох, есть ли какой-либо способ отправить код без сосания?

0 голосов
/ 04 августа 2008

Мои быстрые подсказки.

Вместо:
void strrep(char *input, char *search, char *replace)
попробовать:
void strrep(char *&input, char *search, char *replace)

и чем в теле:
input = realloc(input, strlen(input) + delta);

Обычно читайте о передаче аргументов функции в виде значений / reference и realloc () description:).

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...