2016-12-15 2 views
1

Ich versuche, eine Implementierung von Ringpuffer in Array durchzuführen. Ich behalte meine Daten in der Struktur und verwalte sie mit wenigen Methoden wie Push, Pop usw. Das Programm ist mehr oder weniger funktional und verhält sich wie erwartet, jedoch habe ich in meinem Valgrind-Test Fehler festgestellt. Und ich bin nicht in der Lage herauszufinden, was mit meinem Code nicht stimmt. Obwohl es so aussieht, als würde die Verwaltung von Daten über Zeiger in meiner Struktur das entscheidende Problem darstellen. Ich wäre sehr dankbar, wenn mir jemand in die richtige Richtung zeigen könnte, denn ich bin an diesem Punkt wirklich verloren.Abrufen von Daten vom Zeiger in der Struktur "Ungültiges Lesen/Schreiben"

Dies ist, wie meine Struktur wie folgt aussieht:

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

Hier meine Methoden sind Pufferoperationen zu verwalten:
(kommentiert Code erzeugt so ziemlich die gleichen Fehler wie memcpy)

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; 
    } 

Dies ist meine Hauptmethode, um die Funktionalität der erwähnten Pufferoperationen zu testen:
(Queue.h ist, wo meine Funktionen definiert sind)

#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; 
} 

Und schließlich das ist mein valgrind Ausgabe:

==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) 

Spitzcodezeilen sind:

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

Vielen Dank im Voraus, ich hoffe, jemand wird dazu in der Lage sein Zeig mir, was ist das für eine schlechte Übung, die ich hier mache:/

+0

Vielleicht nicht die Wurzeln verursachen, aber immer noch zu meinem Auge knallt: Das 'int * item = malloc (sizeof (int *));' macht keinen Sinn. 'item' zeigt auf ein' int', also sollte die Anzahl der Bytes, auf die es zeigt, die Größe eines 'int' nicht von einem' int * 'haben. So wollen Sie Zeiger überprüfen, wie ihnen zugeordnet wird und was und wie viel schließlich kopiert wird, auf was sie zeigen. – alk

+0

Vielen Dank, korrigiert, aber der Fehler bleibt bestehen ... Ich werde dies auch in anderen Zuordnungen überprüfen – shade254

Antwort

1

Es gibt ein paar Probleme damit. Erstens sollten Sie die Daten nicht in ein int * umwandeln, da dies ein Zeiger auf irgendetwas sein kann. In Ihrer Strukturdeklaration sollten das Datenarray und alle anderen Zeiger als void ** deklariert werden, da es auf diesen void * -Typ verweist, der im Array gespeichert ist. Sie brauchen überhaupt kein memcpy. Sie ordnen es einfach so zu: *(queue->tail) = data;, wo Daten vom Typ void * sind. Meiner Meinung nach wäre es ein klarer Weg, den Kopf und den Schwanz einfach als ganze Zahlen (als Index relativ zum Array) zu speichern - dann könnten Sie das tun: queue->data[queue->tail] = data;, ohne sich manuell mit den Zeigern auseinandersetzen zu müssen.

Gerade jetzt, was Sie auf diesen Linien tun:

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

ist eine Zuweisung von Speicher, der nie mehr befreit wird, aber wichtig ist, dass Sie nicht tatsächlich sogar den Wert zurückgibt, die in queue- gespeichert wurde> Kopf. Sie geben die Adresse des Speicherblocks zurück, den Sie gerade für den Artikel reserviert haben. Um den Wert zu erhalten, Sie zu dereferenzieren es mit einem Stern haben würde, wie in: return *item; Wieder was Sie wirklich obwohl wollen, ist eine einfache Zuordnung: void *item = *(queue->head);

+0

Sie können die Größe auch mit dem Objektnamen selbst festlegen und sich nie um eine falsche Schriftgröße kümmern. Während es mit Grundtypen trivial ist, ist es sehr vorteilhaft, mit komplex definierten Typen zu arbeiten. Hier wäre es einfach "int * item = malloc (sizeof * item);' Es ist nichts falsch mit der Art, wie Sie es getan haben, aber es lohnt sich, auf die Alternative hinzuweisen. –

0

Basierend auf Signaturen einiger Funktionen in Ihrem Code (insbesondere bool push_to_queue(queue_t *queue, void *data) { ...) I vermuten, dass was Sie wollen ist eine Struktur zum Speichern von Zeigern auf alle gewünschten Daten. Und diese Struktur sollte sich wie eine Warteschlange verhalten. Darüber hinaus werden Sie es als eine zirkuläre Warteschlange implementieren.

Die erste Frage, die ich mit Ihrem Code sehen, ist in der Gestaltung der Warteschlange:

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

Am wichtigsten ist - warum sollten Sie diese Hinweise in einem Array von ganzen Zahlen speichern möchten (in int* data;)? Vielleicht ein Array von Zeigern wäre besser? In C haben Zeiger unabhängig vom Typ die gleiche Größe - sie müssen Speicheradressen speichern können, die auf 64-Bit-Betriebssystemen normalerweise bedeuten, dass sie 8 Byte belegen (8 * 8 = 64). Allerdings empfehle ich Ihnen ein Array von Zeigern zu leeren. Warum? Denn niemand wird jemals von der Tatsache abgelenkt werden, dass Sie i. e. ein Array von Zeigern auf int, weil die Leute denken können, dass Sie tatsächlich Zeiger auf Ganzzahlen speichern. Indem Sie Zeiger auf void verwenden, machen Sie dies für jeden klar, der diesen Code nach Ihnen verwenden wird.

Deshalb empfehlen I eine ähnliche Struktur wie diese zu erstellen:

typedef struct queue_t{ 
    void** base; 
    size_t capacity; 
    size_t used; 
    void** head; 
    void** tail; 
} queue_t; 
  • void** base auf das erste Element des Arrays verweisen.
  • size_t capacity wird die Länge des Arrays gespeichert werden - wie viele Zeiger höchstens dort gespeichert werden können
  • size_t used speichert Anzahl der aktuell gespeicherten void-Zeiger.
  • void** head wird dem nächste verfügbare Array-Element zeigen (so, wenn der Benutzer push aufruft, speichern wir seine data zu *head
  • void** tail den ältesten Elemente des Arrays (so zeigen wird, wenn der Benutzer ruft pop,

    : wir werden Sie können Ihre Struktur erstellen mit Funktion wie diese an einem gewissen Punkt)

Dann

Und schließlich mir zeigen lassen, wie eine Push-Funktion aussehen wird:

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; 
} 

Und mit der gleichen Logik können Sie pop Funktion und andere Sie wollen schreiben.

Verwandte Themen