Ошибка сегментирования стека / толчка неправильных одинаковых номеров - PullRequest
0 голосов
/ 03 января 2019

Я пишу стек, который является связанным списком данных (тип void). Данные, которые я тестирую,

struct my_data {
    int val;
    char name[60];
};

struct my_stack_node {
void *data;
struct my_stack_node *next;
};

struct my_stack {
int size;
struct my_stack_node *first;
};

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

    s1 = my_stack_init(sizeof(struct my_data));
    if (!s1) {
        puts("Error in my_stack_init()");
        exit(1);
    }
    printf("\ns1 initialized, size of data: %lu\n", sizeof(struct my_data));

    for (int i = 0; i < NODES; i++) {
        data = malloc(sizeof(struct my_data)); // We must allocate static memory
        data->val = i;
        sprintf(data->name, "Value %d", i);
        if (my_stack_push(s1, data)) {
            puts("Error in my_stack_push()");
            exit(1);
        }
     } //s1 is the stack we are using here

И толкать их с помощью my_stack_push (s2, data); стек и данные в качестве аргументов.

Моя функция нажатия:

int my_stack_push(struct my_stack *stack, void *data){

        if(stack == NULL && sizeof(data)> 0){
                printf("Null Stack or data size error.\n");
                //la pila debe existir
                return -1;
        }
        else {

        struct my_stack_node *nodeToPush = malloc(sizeof(struct my_stack_node));
                nodeToPush -> data = data;
                if(stack -> first == NULL) {
                        nodeToPush -> next = NULL;
                        stack -> first = nodeToPush;
                }
                else {
                        nodeToPush -> next = stack -> first;
                        stack -> first = nodeToPush;
                }
        }
        return 0;

    }

И моя поп-функция вот такая

void *my_stack_pop(struct my_stack *stack){
        struct my_stack_node *node = stack->first;
        if(stack->first == NULL){
        return 0;
        }
        stack->first = node->next;
        void *ret = node->data;
        free(node);
        return ret;
}

Но в моем основном случае, когда я высовываю их и пытаюсь сравнить их, я получаю ошибку сегментации:

while ((data1 = my_stack_pop(s1))) {
    data2 = my_stack_pop(fs1);
    printf("Node of s1: (%d, %s)\t", data1->val, data1->name);
    printf("Node of fs1: (%d, %s)\n", data2->val, data2->name);
    if (!data2 || data1->val != data2->val || my_strcmp(data1->name, data2->name)) {
        printf("Data in s1 and fs1 are not the same.\n (data1->val: %d <> data2->val: %d) o (data1->name: %s <> data2->name: "
               "%s)\n",
               data1->val, data2->val, data1->name, data2->name);
        exit(1);
    }
    size1 = sizeof(*data1);
    size2 = sizeof(*data2);
    free(data1);
    free(data2);
}
printf("size of data from s1: %d\t", size1);
printf("size of data from fs1: %d\n", size2);

(2 стека являются копиями друг друга, поэтому то, что я сделал, должно быть таким же, как я читал). Когда я возвращаю весь узел в функции pop (не данные, а весь my_stack_node), все верно ... но неправильно:

Comparing the data...

Узел s1: (0, значение 0) // хороший Узел fs1: (0, значение 0) 8 8

Узел s1: (-1203217792, NV) // здесь все начинает идти не так Узел fs1: (-1203217792, NV) 8 8

Узел s1: (-1203217792, NV) Узел fs1: (-1203217792, NV) 8 8

Узел s1: (-1203217792, NV) Узел fs1: (-1203217792, NV) 8 8

Узел s1: (0,) Узел fs1: (0,) двойное освобождение или коррупция (fasttop) Прервано (ядро сброшено)

Размер совпадает с введенными данными, но значение и имя неверны (даже в не скопированном стеке), что должно быть:

New node in s1: (0, Value 0)
New node in s1: (1, Value 1)
New node in s1: (2, Value 2)
New node in s1: (3, Value 3)
New node in s1: (4, Value 4)
New node in s1: (5, Value 5)
New node in s1: (6, Value 6)
New node in s1: (7, Value 7)
New node in s1: (8, Value 8)
New node in s1: (9, Value 9)

Но когда я возвращаю (в моем стеке) сами данные, как в коде, я выгружаю ядро ​​при печати теста (данные, которые имеют длину 8 байтов, как один вход).

Когда я возвращаю узел (размер = 64), он правильно печатает неверные данные, но когда я возвращаю данные (размер = 8 (например, нажатый)), происходит сбой ядра.

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

Похоже, это происходит только тогда, когда я читаю данные2, а не данные1. Это код, который я использую для записи и чтения файла:

Запись:

int my_stack_write(struct my_stack *stack, char *filename){
        int count = 0;
        struct my_stack_node *aux =malloc(sizeof(struct my_stack_node));
        FILE *file = fopen(filename, "wb");

        if(stack->first != NULL){
                aux = stack->first;
                count++;
                while(aux->next != NULL){
                        printf("New node in s1: (%p)\n", aux->data);
                        fwrite(aux ,sizeof(struct my_stack_node), 1, file);
                        aux = aux->next;
                        count++;
                }
                printf("New node in s1: (%p)\n", aux->data);
                fwrite(aux ,sizeof(struct my_stack_node), 1, file);

        }
        fclose(file);
        return count;
}

Читать:

struct my_stack *my_stack_read(char *filename){
        struct my_stack *stackRead = my_stack_init(sizeof(struct my_stack_node));
        struct my_stack_node *stackNode = malloc(sizeof(struct my_stack_node));
        FILE *file = fopen(filename, "rb");

        if(!file){
                puts("Impossible obrir el fitxer");
                return NULL;
        }else{

                while(fread(stackNode, sizeof(struct my_stack_node), 1, file)){

                                printf("New node in fs1: (%p)\n", stackNode->data);
                                stackNode = (struct my_stack_node*) stackNode;
                                my_stack_push(stackRead, stackNode->data);
                }
                fclose(file);
                struct my_stack *InvertedStack = my_stack_init(sizeof(struct my_stack_node));
                struct my_stack_node *aux = malloc(sizeof(struct my_stack_node));

                while(my_stack_len(stackRead) !=0){
                printf("Inverting the stack\n");
                aux = my_stack_pop(stackRead);
                my_stack_push(InvertedStack, aux);
                }
                return InvertedStack;
        }
}

Спасибо всем, кто помогает.

MCVE программы, чтобы люди могли проверить весь код и помочь лучше:

test2.c:

https://codeshare.io/244eN4

my_lib.c: https://codeshare.io/G7L8Ab

my_lib.h:

https://codeshare.io/5DzZOm

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

Ответы [ 2 ]

0 голосов
/ 04 января 2019

Разработка хороших, надежных и согласованных API и библиотек - очень сложная работа, особенно в языках программирования, в которых создание интерфейсов несколько сложнее, чем в объектно-ориентированных языках.

Я хочу быть очень хорошим, но код, который вы разместили, имеет утечки памяти, необработанные ошибки, неопределенное поведение и плохой дизайн. Оператор sizeof используется не по назначению. Я могу только догадываться, что вы не понимаете, как на самом деле работает распределение памяти, и какова концепция указателя и общего указателя void*.

Ну, поехали.

Итак, причины, по которым код вызывает ошибку:

  1. Как и предполагалось, данные, на которые указывает стек, недействительны. На самом деле это malloc заявлено, но впоследствии free d через несколько строк:

for (int i = 0; i < NODES; i++) {
    struct ... * data = malloc(sizeof(struct my_data));
    my_stack_push(s1, data); // pushes the data pointer to the stack
    void *data1 = my_stack_pop(s1); // pops the data pointer to the stack
    ...
    assert(data1 == data); // data and data1 are the same
    free(data1); // and data get's freed
    // the memory behind both data and data1 is freed in this point
    // thus the pointer s1.first->node->data is invalid
    // as the code runs in loop, effectively all the data in this stack are invalid
}
  1. Вы удваиваете свободный указатель в main() в цикле while. И s1, и fs1 получены путем вызова my_stack_read для одного и того же файла - таким образом, логически они должны содержать одинаковые значения. Поскольку они хранят одинаковые значения, сохраненные указатели на данные одинаковы, таким образом, освобождение указателя также освободит и сделает недействительным второй указатель во втором списке. Двойное освобождение - неопределенное поведение, и я предполагаю, что это должно привести к чему-то похожему на ошибку сегментации в нормальных системах.

while ((data1 = my_stack_pop(s1))) {
    data2 = my_stack_pop(fs1);
    ...
    assert(data1 == data2); // same pointers
    free(data1);
    free(data2); // double free corruption - seg fault
}
  • После исправления ошибок запустится код и выведет «Все тесты пройдены», доступна живая версия здесь . В любом случае ниже приведены некоторые заметки:

    1. Нет необходимости выделять массив стеков в my_stack_init:

struct my_stack *my_stack_init(int size){
    struct my_stack *stack = malloc(sizeof(struct my_stack_node) * size);
    ...

stack теперь указывает на size количество sizeof(struct my_stack_node) байтов памяти. Вам нужна только одна структура my_stack_node. Также sizeof возвращает size_t. Лучшая версия будет:

struct my_stack *my_stack_init(size_t size){
    struct my_stack *stack = malloc(sizeof(struct my_stack_node));
    ...
  1. my_stack_read утечки памяти:

struct my_stack_node *aux = malloc(sizeof(struct my_stack_node));
...
aux = my_stack_pop(stackRead);
  1. Отступ в коде, который вы разместили, немного отклонен. Попытайтесь сохранить один стиль отступов, я мог бы рекламировать старый добрый стиль кодирования ядра Linux , но вы можете использовать все, что угодно, но будьте последовательны. Также ваш код может использовать некоторую реструктуризацию - ограничение области видимости переменных или, например, удаление else после return NULL может повысить читабельность.

  2. sizeof возвращает size_t. Правильный способ печати size_t - использовать модификатор "%zu" printf. Вы можете печатать указатели, приведя к void* и используя "%p" модификатор printf.

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

int my_stack_push(struct my_stack *stack, void *data) {
   ...
   // allocate memory for new link in the list
   struct my_stack_node *nodeToPush = malloc(sizeof(struct my_stack_node));
   if (nodeToPush == NULL) abort();
   // allocate memory for data itself
   nodeToPush->data = malloc(stack->size);
   if (nodeToPush->data == NULL) abort();
   // memory copy the data into the pointer
   memcpy(nodeToPush->data, data, stack->size);
   ..

В такой реализации стек отвечает за освобождение указателя и хранит только копии данных. Таким образом, все ручки должны быть переписаны, чтобы поддержать это. Размер данных доступен через stack->size и инициализируется в my_stack_init с аргументом.

  1. Хранение значения указателей в файле кажется плохой идеей с точки зрения сериализации. Значения указателя меняются между прогонами. Хранение указателей на элементы списка выглядит очень плохо. Лучше хранить память о самих данных, я не вижу смысла хранить значения указателей в списке. Обратите внимание, что в текущей реализации значение stackNode->next действительно не используется в my_stack_read, потому что это значение уже было освобождено ранее. Я не понимаю, почему вы даже записываете значение stackNode->next в файл, если вы никогда не используете его.

Таким образом, мы можем хранить сами данные в файле:

int my_stack_write(struct my_stack *stack, char *filename){
    ...
    node = stack->first;
    void *data = node->data;
    // write the data behind the node to the file
    if (fwrite(data, stack->size, 1, file) != 1) {
         return -100;
    }
}

Подобным образом мы могли бы переписать my_stack_read:

struct my_stack *my_stack_read(char *filename, size_t size) {
     ...
     struct my_stack *stack = stack_init(size);
     ...
     void *newdata = malloc(stack->size);
     while (fread(newdata, stack->size, 1, file)) {
           my_stack_push(stack, newdata);
     }
     free(newdata); // as in my proposed implementation my_stack stores copy
     // of the memory behind the pointers, we can safely manage own memory
     // by freeing the pointer

}
0 голосов
/ 03 января 2019

у вас проблемы в my_stack_pop

void *ret =  malloc(sizeof(struct my_stack_node));
ret = node->data;

malloc бесполезен (и создает утечку памяти), и вам не хватает свободного узла тоже

Вы можете заменить эти 2 строки на:

void * ret = node->data;

free(node);

Другие замечания

  • в my_stack_push проверьте ошибку перед , чтобы выполнить выделение, или освободите nodeToPush в случае ошибки, если у вас есть утечка памяти
  • sizeof(x) где x - это void* всегда будет иметь значения 4, если вы используете 32-битный процессор, и 8, если это 64-битный процессор. sizeof - это не strlen , например

Наконец, относительно 2 стека являются копиями друг друга, поэтому то, что я сделал, должно быть одинаковым вам трудно помочь, потому что вы не говорите, как вы клонировали стек


(замечания после редактирования)

В my_stack_write

  • не инициализируйте aux с выделением, у вас снова есть утечка памяти, делающая это.
  • выгрузка памяти из my_stack_node не работает, ваша цель - сохранить данные (содержит my_data), а не ячейку, указывающую на данные

В my_stack_read

  • Бесполезно использовать динамическое выделение для my_stack_node, его можно поместить в стек (struct my_stack_node stackNode;), иначе не забудьте освободить его, потому что вы снова вносите утечку памяти.
  • та же ошибка при чтении my_stack_node, вы должны прочитать сохраненные данные (a my_data)
  • stackNode = (struct my_stack_node*) stackNode;, который ничего не делает, потому что он устанавливает stackNode с собой
  • my_stack_push(stackRead, stackNode->data); не имеет ожидаемого результата, поскольку stackNode->data имеет неверное значение, прочитанное в файле.

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

...