Проблема в том, что вы освобождаете указатель, который, возможно, уже был освобожден, и вы не знаете, сколько места используется не имеете указателя на последнее выделенное пространство (в общем) чтобы точно не освободить память.В main()
у вас есть:
char **images_filenames;
[...]
if ((images_filenames = ((char**) malloc(10 * sizeof (char*)))) == NULL) {
[...]
if (readInputFile(fpin, images_filenames) == -1) {
[...]
deallocate2D(images_filenames, num_lines);
Вы выделяете 10 символьных указателей, а затем передаете этот массив в функцию readInputFile()
.Внутри этой функции есть код для перераспределения массива, но вы не предоставили основной программе возможность узнать, что это за новый адрес.Это можно сделать, передав указатель на то, что вы хотите изменить, или у вас есть функция, возвращающая измененное значение (или вы прибегаете к отвратительным практикам, таким как использование глобальных переменных вместо параметров - но вы не должны этого делать).
Итак, вам нужно:
if (readInputFile(fpin, &images_filenames) == -1) {
И в функции readInputFile()
вам нужно много изменений - большое, чтобы иметь дело с аргументом тройного указателя,а затем множество проблем с кодированием:
int readInputFile(FILE *fp, char ***ppp_files)
{
num_lines = 0;
int s = 10;
char line[MAX_LENGTH];
char **file_images = *ppp_files;
char **final_filenames;
Обновление: Я не заметил, что это просто инициализация num_lines, а не ее объявление.Таким образом, num_lines должна быть какой-то глобальной переменной ... некоторые комментарии ниже необходимо скорректировать, чтобы учесть это.
Пока что изменение (почти) тривиально;мы получаем указатель на 'char **', отсюда и тройной аргумент указателя.Чтобы упростить следующий код, создайте локальную копию параметра под старым именем (file_images
) и инициализируйте его значением, на которое указывает аргумент.Следующий код может продолжить работу с file_images
;просто убедитесь, что аргумент обновлен перед возвратом.
Кроме ...
Вы предполагаете, что '= 10', но на самом деле вы должны заставить функцию main сообщать вам, сколько строк былоимеется в наличии.Было выделено 10 строк, но это не ясно без тщательного изучения, которое имело место.У вас должна быть программа main()
, сообщающая, сколько строк было предварительно выделено - дополнительный аргумент функции.Вы также сталкиваетесь с проблемой, заключающейся в том, что программа main()
не может сообщить функции deallocate2D()
, сколько строк в массиве, потому что она не знает.Не ясно, как ваш код компилируется;у вас есть локальная переменная num_lines
, но есть ссылка на переменную num_lines
в main()
, для которой нет объявления.Локальная переменная маскирует любую глобальную переменную.
while (fgets(line, sizeof line, fp) != NULL) {
if (line[0] != '\n') {
if (num_lines >= s) {
s += 100;
Добавление большого количества строк - хорошая идея;он «амортизирует» стоимость перераспределений.
if ((file_images = (char**) realloc(file_images, s * sizeof (char*))) == NULL)
Однако существуют определенные проблемы с техникой, которую вы использовали.Стиль чистого кода: если строка содержит if
со встроенным назначением и она становится слишком длинной, разбейте назначение перед условием:
file_images = (char**) realloc(file_images, s * sizeof (char*));
if (file_images == NULL)
Теперь осталась лишь небольшая ошибка.Что произойдет, если realloc()
не удастся ...
Правильно, вы вытекли из памяти, поскольку значение в file_images
равно нулю, поэтому нет способа освободить то, на что оно указывало. Никогда не пишите :
x = realloc(x, size);
Утечка памяти при сбое!Следовательно, вам необходимо:
char **new_space = realloc(file_images, s * sizeof (char*));
if (new_space == NULL)
{
printf("Error reallocating space for 2d array: %s\n",
strerror(errno));
*ppp_files = file_images;
return -1;
}
}
Как правило, сообщения об ошибках должны быть напечатаны на stderr
;Я не исправил это.
Обратите внимание, что я тщательно скопировал последнее (ненулевое) значение file_images
в переменную в основной программе.Может быть целесообразным сделать то же самое с размером (другое изменение интерфейса) или использовать структуру для инкапсуляции размера массива и указателя на его базу.
if ((file_images[num_lines] = malloc(MAX_LENGTH * sizeof (char))) == NULL)
{
printf("Error allocating space for 2d array: %s\n", strerror(errno));
return -1;
}
Эту ошибку необходимо установить*ppp_files = file_images;
.
strncpy(file_images[num_lines], line, MAX_LENGTH);
if (file_images[num_lines] == NULL) {
printf("Strncpy failed: %s\n", strerror(errno));
return -1;
}
Этот тест нечетный;Вы знаете, что file_images[num_lines]
не равно нулю, и strncpy()
не меняет этого.Вам не нужен тест и обработка ошибок.
printf("name of file %d is: %s \n", num_lines, file_images[num_lines]);
num_lines++;
}
}
printf("Num_lines: %d\n",num_lines);
OK ...
//realloc to number of lines in the file, to avoid wasting memory
Приятное прикосновение.Это едва стоит того;даже на 64-битной машине вы тратите не более 1 КиБ максимум.Тем не менее, ничего плохого в том, чтобы быть аккуратным - хорошо.
if ((final_filenames = realloc(file_images, num_lines * sizeof (char*))) == NULL) {
printf("Error reallocating space for 2d array: %s\n", strerror(errno));
return -1;
Опять же, вам нужно установить *ppp_files = file_images;
до возвращения.
} else {
file_images = final_filenames;
Это не влияет на значение в программе main()
.Это должно быть снова *ppp_files = file_images;
.
deallocate2D(final_filenames, num_lines);
Держись - ты освобождаешь все свое тщательно отведенное место?Таким образом, вы не собираетесь использовать его в конце концов?Приведенное выше присваивание просто скопировало значение указателя вокруг;он не сделал копию памяти ...
}
return 0;
//don't forget to free lines 2d array! (here or at the end of the code)
}
Этот комментарий неправильный - при успешном возврате память уже освобождена.не используйте 'vim' или другую производную 'vi' для редактирования.Люди, у которых есть открывающая скобка их функций в столбце 1, потому что тогда вы можете переходить вперед или назад по файлу к началу следующей или предыдущей функции, используя ']]
' или '[[
'.Работать с кодом там, где это не работает, утомительно.
Ну, это начальная диагностика ... Вот рабочий код, использующий структуру для передачи массива имен файлов.Я оставил тело функции readInputFile()
, используя локальные переменные, которые копируются из структуры, и обеспечил постоянное обновление структуры.
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
enum { MAX_LENGTH = 512 };
typedef struct FileNameArray
{
size_t nfiles; /* Number of file names allocated and in use */
size_t maxfiles; /* Number of entries allocated in array */
char **files; /* Array of file names */
} FileNameArray;
static void deallocate2D(FileNameArray *names)
{
for (size_t i = 0; i < names->nfiles; i++)
free(names->files[i]);
free(names->files);
names->nfiles = 0;
names->files = 0;
names->maxfiles = 0;
}
static int readInputFile(FILE *fp, FileNameArray *names)
{
int num_lines = names->nfiles;
int max_lines = names->maxfiles;
char **file_names = names->files;
char line[MAX_LENGTH];
char **final_filenames;
while (fgets(line, sizeof line, fp) != NULL)
{
if (line[0] != '\n')
{
/* Remove newline from end of file name */
char *nl = strchr(line, '\n');
if (nl != 0)
*nl = '\0';
if (num_lines >= max_lines)
{
max_lines += 100;
char **space = realloc(file_names, max_lines * sizeof (char*));
if (space == NULL)
{
fprintf(stderr, "Error reallocating space for 2d array: %s\n",
strerror(errno));
return -1;
}
names->maxfiles = max_lines;
names->files = space;
file_names = space;
}
if ((file_names[num_lines] = malloc(strlen(line) + 1)) == NULL)
{
fprintf(stderr, "Error allocating space for 2d array: %s\n",
strerror(errno));
return -1;
}
names->nfiles++;
strcpy(file_names[num_lines], line);
printf("name of file %d is: %s \n", num_lines, file_names[num_lines]);
num_lines++;
}
}
printf("Num_lines: %d\n", num_lines);
//realloc to number of lines in the file, to avoid wasting memory
if ((final_filenames = realloc(file_names, num_lines * sizeof (char*))) == NULL)
{
fprintf(stderr, "Error reallocating space for 2d array: %s\n",
strerror(errno));
return -1;
}
names->maxfiles = num_lines;
names->files = final_filenames;
return 0;
}
int main(int argc, char *argv[])
{
FileNameArray names = { 0, 0, 0 };
//check parameters
if (argc < 4)
{
fprintf(stderr, "Usage: %s input_filename.ppm charWidth charHeight\n",
argv[0]);
return -1;
}
printf("Opening input file [%s]\n", argv[1]);
FILE *fpin = fopen(argv[1], "r");
if (fpin == NULL) {
fprintf(stderr, "Could not open input file %s (%s)\n",
argv[1], strerror(errno));
return -1;
}
if ((names.files = malloc(10 * sizeof (char*))) == NULL)
{
fprintf(stderr, "Error allocating initial space for 2d array: %s\n",
strerror(errno));
return -1;
}
names.maxfiles = 10;
if (readInputFile(fpin, &names) == -1)
{
fprintf(stderr, "Error reading image filenames from input\n");
return -1;
}
fclose(fpin);
printf("###########\n");
deallocate2D(&names);
printf("Done!\n");
return 0;
}