0

Имеется следующая функция:

int save_arg(int **array_arg, int index) {
    static int counter = 0;
    static int arr_size = 5 * sizeof(int);
    static int n_memb = 5;
    if (counter == 0) {
        *array_arg = calloc(n_memb, arr_size);
    } else {
        if(counter == n_memb){
            n_memb += 5;
            arr_size += arr_size;
            *array_arg = realloc(*array_arg, arr_size);
        }
    }
    **(array_arg + counter) = index;
    counter++;
    return counter;
}

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

Проблема: При обращении к массиву array_arg вне функции возникает segf, внутри функции работает стабильно.

Паттерн кода, по которому будет использоваться функция:

 int *array = NULL;
    for (int i = 0; i < 10; i++) {
        save_arg(&array, i);
    }
    for (int i = 0; i < 10; i++) {
        printf("%i", *(array + i));
    }

5
  • **(array_arg + counter) = index; - здесь вы пишете мимо памяти. Вероятно вы её портите. Падает всё позже. 21 ноя 2022 в 6:42
  • calloc выделяет больше памяти чем нужно. arr_size растёт быстрее чем вы его используете. Оба недочёта не приводят к сбоям, но получается неаккуратно. 21 ноя 2022 в 6:43
  • Дизайн со статиками не красивый. Почему бы не объединить array_arg, counter и nmemb в одну структуру и не работать с ней? Код по сути будет тот же, а вы получите самодостаточный контейнер. Вы думали, что будет если обратиться к вашей функции с разными массивами? 21 ноя 2022 в 6:47
  • 1
    Изменил строку : **(array_arg + counter) = index; на *(*array_arg + counter) = index;, вроде не падает.
    – Flex1sh
    21 ноя 2022 в 6:55
  • Про структуру и передачу разных массивов не задумывался, но мне придется это реализовать, так как одновременно буду использовать функцию с 2 массивами, огромное спасибо!
    – Flex1sh
    21 ноя 2022 в 6:59

1 ответ 1

2

Вы правильно догадались исправить:

<    **(array_arg + counter) = index;
--------
>    *(*array_arg + counter) = index;

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

Теперь код работает и не падает. Но есть ещё пара мест, которые можно улучшить.

arr_size и n_memb - дублируют друг друга. Размер массива в байтах должен быть пропорционален числу элементов в массиве. Но arr_size растёт значительно быстрее чем n_memb. Например, если n_memb == 105, то arr_size == 20971520. Слишком много. Если убрать дублирование, то получится:

int save_arg(int **array_arg, int index) {
    static int counter = 0;
    static int n_memb = 5;
    if (counter == 0) {
        *array_arg = calloc(n_memb, sizeof(**array_arg));
    } else {
        if(counter == n_memb){
            n_memb += n_memb;
            *array_arg = realloc(*array_arg, n_memb * sizeof(**array_arg));
        }
    }
    *(*array_arg + counter) = index;
    counter++;
    return counter;
}

*p = realloc(*p, ...) - известная ошибка. Когда памяти достаточно, всё хорошо. Если памяти не хватит, realloc вернёт NULL, указатель обнулится и старая память утечёт. Исправляем:

int save_arg(int **array_arg, int index) {
    static int counter = 0;
    static int n_memb = 5;
    if (counter == 0) {
        *array_arg = calloc(n_memb, sizeof(**array_arg));
    } else {
        if (counter == n_memb) {
            int new_n_memb = 2 * n_memb;
            int *tmp = realloc(*array_arg, new_n_memb * sizeof(**array_arg));
            if (tmp == NULL) {
                return -1; // какой-то признак ошибки
            }
            n_memb = new_n_memb;
            *array_arg = tmp;
        }
    }
    *(*array_arg + counter) = index;
    counter++;
    return counter;
}

Отдельная обработка пустого массива не нужна и calloc не нужен. Можно обойтись одним realloc. Работает чуть быстрее, так как с верхнего уровня ушла почти бесполезная проверка counter == 0:

int save_arg(int **array_arg, int index) {
    static int counter = 0;
    static int n_memb = 0;
    if (counter == n_memb) {
        int new_n_memb = (n_memb == 0) ? 5 : 2 * n_memb;
        int *tmp = realloc(*array_arg, new_n_memb * sizeof(**array_arg));
        if (tmp == NULL) {
            return -1; // какой-то признак ошибки
        }
        n_memb = new_n_memb;
        *array_arg = tmp;
    }
    *(*array_arg + counter) = index;
    counter++;
    return counter;
}

Функцию нельзя вызывать для разных массивов, так как её внутреннее состояние сильно связано с конкретным одним массивом. Исправляем:

typedef struct {
    int n_memb;
    int counter;
    int *array;
} args_t;

int save_arg(args_t *args, int index) {
    if (args->counter == args->n_memb) {
        int n_memb = (args->n_memb == 0) ? 5 : 2 * args->n_memb;
        int *tmp = realloc(args->array, n_memb * sizeof(*args->array));
        if (tmp == NULL) {
            return -1; // какой-то признак ошибки
        }
        args->n_memb = n_memb;
        args->array = tmp;
    }
    *(args->array + args->counter) = index;
    args->counter++;
    return args->counter;
}

Использование:

args_t args = {0};
for (int i = 0; i < 10; i++) {
    save_arg(&args, i);
}
for (int i = 0; i < 10; i++) {
    printf("%i", *(args.array + i));
}

Осталось отрефакторить и получится то что у меня всегда получается в подобных случаях:

typedef struct {
    int size;
    int capacity_;
    int *array;
} args_t;

void args_t_clear(args_t *args) {
    free(args->array);
    args->size = 0;
    args->capacity_ = 0;
    args->array = NULL;
}

bool args_t_reserve(args_t *args, int capacity) {
    if (capacity > args->capacity_) {
        int *tmp = realloc(args->array, capacity * sizeof(*args->array));
        if (tmp == NULL) {
            return false;
        }
        args->capacity_ = capacity;
        args->array = tmp;
    }
    return true;
}

bool args_t_append(args_t *args, int index) {
    if (args->size == args->capacity_) {
        int capacity = (args->capacity_ == 0) ? 5 : 2 * args->capacity_;
        if (!args_t_reserve(args, capacity)) {
            return false;
        }
    }
    args->array[args->size] = index;
    ++args->size;
    return true;
}

int main() {
    args_t args = {0};
    for (int i = 0; i < 10; ++i) {
        args_t_append(&args, i);
    }
    for (int i = 0; i < args.size; ++i) {
        printf("%i", args.array[i]);
    }
    args_t_clear(&args);
}
1
  • Воу, спасибо за ревью, многое подчерпнул для дальнейшей работы.На структуру я переписал, но теперь calloc вырежу и дыру заделаю.
    – Flex1sh
    21 ноя 2022 в 15:42

Ваш ответ

Нажимая на кнопку «Отправить ответ», вы соглашаетесь с нашими условиями использования и подтверждаете, что прочитали и поняли наши политику конфиденциальности и нормы поведения.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.