2016-12-15 16 views
1

Я пытаюсь выполнить реализацию циклического буфера в массиве. Я сохраняю свои данные в структуре и управляю ими несколькими способами, такими как push, pop и т. Д. Программа более или менее функциональна и ведет себя так, как ожидалось, однако я сталкиваюсь с ошибками в моем испытании valgrind. И я не в состоянии выяснить, что не так с моим кодом. Хотя кажется, что управление данными через указатели в моей структуре является решающей проблемой. Я был бы очень благодарен, если бы кто-нибудь мог указать мне в правильном направлении, потому что я действительно потерялся в этот момент.Получение данных из указателя в структуре «Недопустимое чтение/запись»

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

typedef struct queue_t{ 
    int* data; 
    int* end; 
    int* head; 
    int* tail; 
    int max_length; 
    int cur_length; 
} queue_t; 

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

int* increase(int* point, queue_t* queue){ 
    if(point != queue->end){ 
     point = point + sizeof(int*); 
     return point; 
    }else{ 
     return queue->data; 
    } 
} 

    queue_t* create_queue(int capacity){ 
     queue_t* fifo; 
     fifo = malloc(sizeof(queue_t)); 
     fifo->data = malloc((capacity) * sizeof(int*)); 
     fifo->end = fifo->data + (capacity*sizeof(int*)); 
     fifo->head = fifo->data; 
     fifo->tail = fifo->data; 
     fifo->cur_length = 0; 
     fifo->max_length = capacity; 
     return fifo; 
    } 

    void delete_queue(queue_t *queue){ 
     free(queue->data); 
     free(queue); 
    } 

    bool push_to_queue(queue_t *queue, void *data){ 
     int *temp = (int*) data; 
     //*(queue->tail) = *temp; 
     memcpy(queue->tail, temp, sizeof(int)); 
     free(data); 
     if(queue->max_length != queue->cur_length){ 
      queue->cur_length++; 
     } 

     queue->tail = increase(queue->tail, queue); 

     if(queue->tail == queue->head){ 
      queue->head = increase(queue->head, queue); 
     } 
     return true; 
    } 

    void* pop_from_queue(queue_t *queue){ 
     if(queue->cur_length == 0){ 
      return NULL; 
     } 
     int *item = malloc(sizeof(int*)); 
     //*item = *(queue->head); 
     memcpy(item, queue->head, sizeof(int)); 
     queue->head = increase(queue->head, queue); 
     queue->cur_length--; 
     return item; 
    } 

Это мой основной метод проверки функциональности упомянутых операций с буфером:
(queue.h где мои функции определены)

#include "queue.h" 


void print_int(void* p){ 
    if(p != NULL){ 
     printf("%d\n", *((int*)p)); 
    } else { 
     printf("NULL\n"); 
    } 
} 

int main(){ 
    int n = 2; 
    int max = 10; 
    queue_t *q; 


    q = create_queue(n); 

    for(int i = 0; i<max;i++){ 
     int* p = malloc(sizeof(int)); 
     *p = i; 
     if(!push_to_queue(q, (void*)p)){ 
      free(p); 
      exit(101); 
     } 
    } 

    for(int i = 0;i<max;i++){ 
     void* p = pop_from_queue(q); 
     print_int(p); 
     free(p); 
    } 
    delete_queue(q); 

    return 0; 
} 

И, наконец, это мой выход Valgrind:

==20293== HEAP SUMMARY: 
==20293==  in use at exit: 0 bytes in 0 blocks 
==20293== total heap usage: 15 allocs, 15 frees, 1,136 bytes allocated 
==20293== 
==20293== All heap blocks were freed -- no leaks are possible 
==20293== 
==20293== ERROR SUMMARY: 7 errors from 2 contexts (suppressed: 0 from 0) 
==20293== 
==20293== 1 errors in context 1 of 2: 
==20293== Invalid read of size 4 
==20293== at 0x40097C: pop_from_queue (queue.c:72) 
==20293== by 0x400713: main (main.c:30) 
==20293== Address 0x52030f0 is 16 bytes before a block of size 4 free'd 
==20293== at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==20293== by 0x4008B8: push_to_queue (queue.c:51) 
==20293== by 0x4006D5: main (main.c:23) 
==20293== Block was alloc'd at 
==20293== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==20293== by 0x4006B5: main (main.c:21) 
==20293== 
==20293== 
==20293== 6 errors in context 2 of 2: 
==20293== Invalid write of size 4 
==20293== at 0x4008AB: push_to_queue (queue.c:50) 
==20293== by 0x4006D5: main (main.c:23) 
==20293== Address 0x52030d0 is 16 bytes after a block of size 16 alloc'd 
==20293== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==20293== by 0x4007FB: create_queue (queue.c:33) 
==20293== by 0x40069E: main (main.c:18) 
==20293== 
==20293== ERROR SUMMARY: 7 errors from 2 contexts (suppressed: 0 from 0) 

Остроконечные строки кода:

72: memcpy(item, queue->head, sizeof(int)); 
50: memcpy(queue->tail, temp, sizeof(int)); 

Большое спасибо, я надеюсь, что кто-то wi я смогу показать мне, что это за плохая практика, которую я здесь делаю:/

+0

Возможно, не из-за корней, но все же появляется на моих глазах: Это 'int * item = malloc (sizeof (int *));' на самом деле не имеет смысла. 'item' указывает на' int', поэтому количество байтов, на которое указывает точка, должно быть размером 'int' not' int * '. Таким образом, вы хотите удвоить контрольные указатели, как их распределить и что и как, в конце концов, оно скопировано на то, на что они указывают. – alk

+0

Большое спасибо, исправлено, но ошибка сохраняется ... Я проверю это и на другие распределения, а также – shade254

ответ

1

Есть несколько проблем с этим. Во-первых, вы не должны передавать данные в int *, потому что это может быть указатель на что угодно. В объявлении структуры массив данных и все остальные указатели должны быть объявлены как void **, поскольку он указывает на этот тип void *, который хранится в массиве. Вам вообще не нужна memcpy. Вы просто назначаете его следующим образом: *(queue->tail) = data;, где данные имеют тип void *. На мой взгляд, более понятным способом было бы просто сохранить голову и хвост целыми числами (как индекс по отношению к массиву), - тогда вы могли бы сделать это: без необходимости обращаться с указателями вручную.

Прямо сейчас, что вы делаете на этих линиях:

int *item = malloc(sizeof(int*)); 
memcpy(item, queue->head, sizeof(int)); 

выделяет часть памяти, которая никогда не получает освобожденную, но что более важно, вы не на самом деле даже возвращающуюся значение, которое было сохранено в queue-> глава. Вы возвращаете адрес блока памяти, который вы только что выделили для элемента. Для того, чтобы получить значение, вы должны разыменования его с звездой, как: return *item; Опять же, что вы действительно хотите, хотя это простое задание: void *item = *(queue->head);

+0

. Вы также можете установить размер с помощью самого имени объекта и никогда не беспокоиться о неправильном типе. Хотя это тривиально с базовыми типами, очень полезно работать со сложными определенными типами. Здесь это просто будет 'int * item = malloc (sizeof * item);' Нет ничего плохого в том, как вы это сделали, но стоит указать на альтернативу. –

0

на основе сигнатур некоторых функций в коде (особенно bool push_to_queue(queue_t *queue, void *data) { ...) I что что вы хотите - это структура для хранения указателей на любые данные, которые вы хотите. И эта структура должна вести себя как очередь. Кроме того, вы собираетесь реализовать его как круглую очередь .

Первый вопрос, который я вижу с вашим кодом в дизайне очереди:

typedef struct queue_t{ 
    int* data; 
    int* end; 
    int* head; 
    int* tail; 
    int max_length; 
    int cur_length; 
} queue_t; 

Самое главное - почему вы хотели бы хранить эти указатели в массив целых чисел (в int* data;)? Может быть, массив указателей был бы лучше? В C указатели имеют одинаковый размер независимо от типа, на который они указывают, - они должны быть способны хранить любой адрес памяти, который в 64-разрядных операционных системах обычно означает, что они занимают 8 байтов (8 * 8 = 64). Тем не менее, я рекомендую вам массив указателей для void. Зачем? Потому что никто не будет отвлекаться на то, что вы используете i. е. массив указателей на int, потому что это может заставить людей думать, что вы на самом деле храните указатели на целые числа - с помощью указателей на void вы делаете это абсолютно ясным для всех, кто будет использовать этот код после вас.

Поэтому я рекомендую создать структуру, подобную этой:

typedef struct queue_t{ 
    void** base; 
    size_t capacity; 
    size_t used; 
    void** head; 
    void** tail; 
} queue_t; 
  • void** base будет указывать на первый элемент массива.
  • size_t capacity сохранит длину массива - сколько указателей на большинство из них может храниться там
  • size_t used будет хранить число в настоящее время хранятся недействительных указателей.
  • void** head будет указывать на следующий элемент доступен массив (так что, когда пользователь звонит push, мы будем хранить его data в *head
  • void** tail будет указывать на старых элементов массива (так что, когда пользователь вызывает pop, мы return *tail; в какой-то момент)

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

queue_t* create_queue(size_t capacity) { 
    queue_t* nq = malloc(sizeof(queue_t)); 
    // Let's allocate the array of pointers to void: 
    nq->base = malloc(sizeof(void*) * capacity); 
    nq->capacity = capacity; 
    nq->used = 0; 
    nq->head = nq->tail = nq->base; 
    return nq; 
} 

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

bool push(queue_t* queue, void* data) { 
    if(queue == NULL || (queue->used == queue->capacity)) 
      return false; 
    *(queue->head++) = data; // this is equivalent to *(queue->head) = data; queue->head += 1; 
    if(queue->head >= queue->base + queue->capacity) 
      queue->head = queue->base; // We went to far, so we go back. 
    return true; 
} 

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