malloc в последующем вызове функции - PullRequest
1 голос
/ 10 ноября 2011

Определено в файле h:

char *copiedText;

Определено в файле c:

char *copyText(char *text){
  if (copiedText != 0) free(copiedText);
  copiedText = (char *)calloc((strlen(text) + 1) * sizeof(char), sizeof(char));
  strcpy(copiedText, text);
  return copiedText;
}

Во-первых, речь идет не о том, как копировать текст, я просто выбрал это в качестве примера. Мой вопрос о free () перед calloc ().

Первый вопрос, который я слышу, вы спрашиваете, почему не просто free () в подходящее время - т.е. когда copiedText больше не нужен?

Короче говоря, я делаю часть программы, и я не могу доверять пользователям моей функции должным образом free () copiedText, поэтому я хочу, чтобы в моей функции было как можно больше кода. Все, что они делают, это включают h и c file и вызывают функцию copyText.

Я знаю, что между последним вызовом copyText и завершением программы будет (небольшая) утечка памяти, но я согласен с этим компромиссом, поскольку я использую только malloc () для небольших объемов данных.

Вопрос заключается в том, освободит ли free () память, выделенную моим calloc (), при таком кодировании?

Ответы [ 5 ]

3 голосов
/ 10 ноября 2011

Обратной стороной является то, что теперь:

  1. указатель, возвращаемый copyText(), действителен только до следующего вызова copyText() - , что вовсе не очевидно дляcaller и, вероятно, стоит документировать как можно лучше;
  2. функция не является поточно-ориентированной.

Лучшим подходом может быть требование явного free()(возможно, оборачивая его в функцию с именем freeText(), в качестве аналога copyText()).

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

1 голос
/ 10 ноября 2011

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

char *copyText(const char *text)
{
   return strcpy(malloc((strlen(text) + 1)), text);
}
1 голос
/ 10 ноября 2011

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

 main(){
    char *a=copyText("Hello, ");
    char *b=copyText("World!\n");
    printf("%s%s",a, b);
}

Конечно, не работает, как ожидалось.

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

1 голос
/ 10 ноября 2011

Что не так с strdup()?calloc() кажется бесполезным, потому что вы сразу же перезаписываете память тем, что есть в copiedText.Вы фактически «дважды инициализируете», что является пустой тратой времени.

Что касается free(), я не уверен, что вы имеете в виду?Память будет свободной, однако она может или не может быть освобождена в системе из-за фрагментации памяти.Я действительно не стал бы беспокоиться об этом ...

0 голосов
/ 10 ноября 2011

я думаю простой

char *copyText(char *text)
{
 if (copiedText != NULL)
   free(copiedText);
copiedText = strdup(text);
return copiedText;
}

должно работать на то, что вы хотите, но, как сказал Экс, все указатели из предыдущего вызова больше не будут существовать .

...