Przeglądaj źródła

Fix segfaults and clear memory leaks.

Stefano Cossu 3 lat temu
rodzic
commit
829bfaecfe
6 zmienionych plików z 184 dodań i 17 usunięć
  1. 3 3
      Makefile
  2. 11 3
      src/graph.c
  3. 9 6
      src/htable.c
  4. 1 2
      test.c
  5. 2 3
      test/test_graph.c
  6. 158 0
      test/test_htable.c

+ 3 - 3
Makefile

@@ -11,7 +11,7 @@ check:
 build:
 	gcc -g -Wall \
 		-std=gnu99 \
-		-Iinclude -Iext/xxHash -Iext/klib \
+		-Iinclude -Iext/xxHash \
 		-luuid \
 		ext/xxHash/xxhash.c src/*.c \
 		-o bin/lsup_rdf.so
@@ -20,7 +20,7 @@ test:
 	gcc -g -Wall \
 		-std=gnu99 \
 		-DDEBUG \
-		-Iinclude -Iext/xxHash -Iext/klib -Itest \
+		-Iinclude -Iext/xxHash -Itest \
 		-luuid \
 		ext/xxHash/xxhash.c src/*.c test.c \
 		-o bin/test
@@ -28,7 +28,7 @@ test:
 profile:
 	gcc -g -Wall \
 		-std=gnu99 \
-		-Iinclude -Iext/xxHash -Iext/klib -Itest \
+		-Iinclude -Iext/xxHash \
 		-luuid \
 		ext/xxHash/xxhash.c src/*.c test.c \
 		-o bin/profile

+ 11 - 3
src/graph.c

@@ -232,12 +232,13 @@ int
 LSUP_graph_add_triple(LSUP_Graph *gr, const LSUP_Triple *spo)
 {
     LSUP_SerTerm sspo[3];
+    LSUP_SerTerm *sterm;
 
     LSUP_term_serialize(spo->s, sspo);
     LSUP_term_serialize(spo->p, sspo + 1);
     LSUP_term_serialize(spo->o, sspo + 2);
 
-    LSUP_TripleKey spok = {0, 0, 0};
+    LSUP_TripleKey spok = NULL_TRP;
 
     // Add term to index.
     for (int i = 0; i < 3; i++) {
@@ -245,8 +246,15 @@ LSUP_graph_add_triple(LSUP_Graph *gr, const LSUP_Triple *spo)
         TRACE("Indexing term key %lu\n", spok[i]);
 
         // If term is already in the index, discard and free it.
-        if (LSUP_htable_put(gr->idx, spok + i, sspo + i) == LSUP_NOACTION)
+        if (LSUP_htable_get(gr->idx, spok + i, NULL) == LSUP_OK) {
+            CRITICAL(sterm = malloc(sizeof(LSUP_Buffer)));
+            //*sterm = sspo[i];
+            sterm = sspo + i;
+            LSUP_htable_put(gr->idx, spok + i, sterm);
+        } else {
+            TRACE("%s", "Term is already indexed.");
             LSUP_buffer_done(sspo + i);
+        }
     }
 
     // Add triple.
@@ -479,7 +487,7 @@ LSUP_graph_free(LSUP_Graph *gr)
         LSUP_Buffer *sterm;
         while(LSUP_htable_iter(
                     gr->idx, &cur, (void**)&spok, (void**)&sterm) == LSUP_OK) {
-            TRACE("Freeing indexed term buffer #%d", cur);
+            TRACE("Freeing indexed term buffer #%d at %p", cur, sterm);
             LSUP_buffer_done(sterm);
         }
 

+ 9 - 6
src/htable.c

@@ -110,8 +110,8 @@ fast_rem32(uint32_t v, uint32_t div, uint64_t divinfo)
 { return v - div * fast_div32(v, div, divinfo); }
 
 
-//static int __attribute__((__unused__))
-static int
+static int __attribute__((__unused__))
+//static int
 validate_psl_p(const HTable *ht, const bucket_t *bucket, unsigned i)
 {
     unsigned base_i = fast_rem32(bucket->hash, ht->size, ht->divinfo);
@@ -127,7 +127,7 @@ HTable *LSUP_htable_new(
         key_hash_fn_t key_hash_fn, key_eq_fn_t key_eq_fn, unsigned flags)
 {
     HTable *ht;
-    CRITICAL(ht = malloc(sizeof(HTable)));
+    CRITICAL(ht = calloc(1, sizeof(HTable)));
 
     ht->ksize = ksize;
     ht->vsize = vsize;
@@ -149,6 +149,8 @@ HTable *LSUP_htable_new(
  */
 int LSUP_htable_resize(HTable *ht, htsize_t newsize)
 {
+    TRACE("Resizing htable to %lu.", (size_t)newsize);
+
     bucket_t *oldbuckets = ht->buckets;
     const htsize_t oldsize = ht->size;
 
@@ -258,8 +260,8 @@ int LSUP_htable_insert(HTable *ht, const void *key, void *val)
      * Found a free bucket: insert the entry.
      */
     TRACE("Inserting {%lu, %lu, %lu} in bucket #%d", entry.key[0], entry.key[1], entry.key[2], i);
-    *bucket = entry; // copy
-    //memcpy(bucket, &entry, sizeof(bucket_t)); // copy
+    //*bucket = entry; // copy
+    memcpy(bucket, &entry, sizeof(bucket_t)); // copy
     ht->nitems++;
 
     ASSERT(validate_psl_p(ht, bucket, i));
@@ -311,7 +313,8 @@ int LSUP_htable_get(const HTable *ht, const void *key, void **valp)
 
         if (ht->key_eq_fn(bucket->key, key, ht->ksize)) {
             // Key found within max probe length.
-            if (valp != NULL) *valp = bucket->val;
+            if (valp != NULL)
+                *valp = bucket->val;
 
             return LSUP_OK;
         }

+ 1 - 2
test.c

@@ -4,8 +4,7 @@
 
 int main(int argc, char **argv) {
 
-    //int result = (term_tests() | htable_tests() | graph_tests());
-    int result = (term_tests() | htable_tests() );
+    int result = (term_tests() | htable_tests() | graph_tests());
 
     printf("Test result: %lu\n", (size_t)result);
 

+ 2 - 3
test/test_graph.c

@@ -76,7 +76,6 @@ static int test_graph_heap()
     LSUP_Graph *gr = LSUP_graph_new(10, "urn:gr:1", LSUP_STORE_MEM);
 
     ASSERT(strcmp(LSUP_graph_uri(gr), "urn:gr:1") == 0, "Graph URI mismatch!");
-    EXPECT_INT_EQ(LSUP_graph_capacity(gr), 16);
     EXPECT_INT_EQ(LSUP_graph_size(gr), 0);
 
     LSUP_graph_free(gr);
@@ -100,7 +99,7 @@ static int test_graph_add()
         printf("OK.\n");
     }
 
-    _free_triples(trp); // gr takes ownership of data.
+    _free_triples(trp); // gr copied data.
 
     EXPECT_INT_EQ(LSUP_graph_size(gr), 8);
 
@@ -142,7 +141,7 @@ static int test_graph_add_100k()
 
 int graph_tests()
 {
-    //RUN(test_graph_heap);
+    RUN(test_graph_heap);
     RUN(test_graph_add);
     RUN(test_graph_add_100k);
     return 0;

+ 158 - 0
test/test_htable.c

@@ -0,0 +1,158 @@
+#include "test.h"
+#include "htable.h"
+
+#define _CT 8
+
+static inline uint64_t id_hash_fn(const void *key, ksize_t size, uint64_t seed)
+{ return *(uint64_t*)key; }
+
+static inline bool buffer_eq_fn(const void *a, const void *b, ksize_t size)
+{ return memcmp(a, b, size) == 0; }
+
+
+static int htable_idx()
+{
+    LSUP_Key keys[_CT] = {5, 8, 13, 21, 34, 55, 89, 5};
+
+    LSUP_HTable *ht = LSUP_htable_new(
+            _CT, sizeof(LSUP_Key), sizeof(LSUP_Buffer),
+            id_hash_fn, buffer_eq_fn, 0);
+
+    LSUP_Buffer values[_CT];
+
+    for (int i = 0; i < _CT; i++) {
+        char tmp[64];
+        sprintf(tmp, "<%lu>", keys[i]);
+        LSUP_buffer_init(values + i, strlen(tmp) + 1);
+        memcpy((values + i)->addr, tmp, strlen(tmp) + 1);
+        printf("Buffer to insert: ");
+        LSUP_buffer_print(values + i);
+
+        if (LSUP_htable_put(ht, keys + i, values + i) != LSUP_OK)
+            LSUP_buffer_done(values + i);
+    }
+
+    EXPECT_INT_EQ(LSUP_htable_size(ht), 7);
+
+    for (int i = 0; i < _CT; i++) {
+        LSUP_Buffer* vtmp;
+        char ptmp[64];
+        LSUP_htable_get(ht, keys + i, (void**)&vtmp);
+
+        printf("Key in get: <%lu>: ", keys[i]);
+        LSUP_buffer_print(vtmp);
+
+        sprintf(ptmp, "<%lu>", keys[i]);
+
+        EXPECT_INT_EQ(memcmp(ptmp, vtmp->addr, vtmp->size), 0);
+    }
+
+    LSUP_Key *ktmp;
+    LSUP_Buffer *vtmp;
+    htsize_t cur = 0;
+
+    while(LSUP_htable_iter(ht, &cur, (void**)&ktmp, (void**)&vtmp) == LSUP_OK) {
+        printf("Key in iter: <%lu>: ", *ktmp);
+        LSUP_buffer_print(vtmp);
+
+        char ptmp[64];
+        sprintf(ptmp, "<%lu>", *ktmp);
+
+        EXPECT_INT_EQ(memcmp(ptmp, vtmp->addr, vtmp->size), 0);
+    }
+
+    cur = 0;
+    while(LSUP_htable_iter(ht, &cur, (void**)&ktmp, (void**)&vtmp) == LSUP_OK) {
+        LSUP_buffer_done(vtmp);
+    }
+
+    printf("Freeing hash table.\n");
+    LSUP_htable_free(ht);
+
+    return 0;
+}
+
+
+static int htable_keys()
+{
+    LSUP_TripleKey keys[_CT] = {
+        {1, 1, 1},
+        {2, 1, 1},
+        {1, 2, 3},
+        {1, 9, 9},
+        {5, 6, 7},
+        {7, 6, 5},
+        {1, 1, 1}, // Duplicate.
+        {2, 1, 1}, // Duplicate.
+    };
+
+    LSUP_HTable *ht = LSUP_htable_new(
+            _CT, sizeof(LSUP_TripleKey), sizeof(LSUP_Buffer),
+            id_hash_fn, buffer_eq_fn, 0);
+
+    LSUP_Buffer values[_CT];
+
+    for (int i = 0; i < _CT; i++) {
+        char tmp[64];
+        sprintf(tmp, "<%lu : %lu : %lu>", keys[i][0], keys[i][1], keys[i][2]);
+        LSUP_buffer_init(values + i, strlen(tmp) + 1);
+        memcpy((values + i)->addr, tmp, strlen(tmp) + 1);
+        TRACE(STR, "Buffer to insert: ");
+        LSUP_buffer_print(values + i);
+
+        if (LSUP_htable_put(ht, keys + i, values + i) != LSUP_OK)
+            LSUP_buffer_done(values + i);
+    }
+
+    EXPECT_INT_EQ(LSUP_htable_size(ht), 6);
+
+    for (int i = 0; i < _CT; i++) {
+        LSUP_Buffer* vtmp;
+        char ptmp[64];
+        LSUP_htable_get(ht, keys[i], (void**)&vtmp);
+
+        printf(
+                "Key in get: <%lu : %lu : %lu>: ",
+                keys[i][0], keys[i][1], keys[i][2]);
+        LSUP_buffer_print(vtmp);
+
+        sprintf(ptmp, "<%lu : %lu : %lu>", keys[i][0], keys[i][1], keys[i][2]);
+
+        EXPECT_INT_EQ(memcmp(ptmp, vtmp->addr, vtmp->size), 0);
+    }
+
+    LSUP_TripleKey *ktmp;
+    LSUP_Buffer *vtmp;
+    htsize_t cur = 0;
+
+    while(LSUP_htable_iter(ht, &cur, (void**)&ktmp, (void**)&vtmp) == LSUP_OK) {
+        printf(
+                "Key in iter: <%lu : %lu : %lu>: ",
+                (*ktmp)[0], (*ktmp)[1], (*ktmp)[2]);
+        LSUP_buffer_print(vtmp);
+
+        char ptmp[64];
+        sprintf(ptmp, "<%lu : %lu : %lu>", (*ktmp)[0], (*ktmp)[1], (*ktmp)[2]);
+
+        EXPECT_INT_EQ(memcmp(ptmp, vtmp->addr, vtmp->size), 0);
+    }
+
+    cur = 0;
+    while(LSUP_htable_iter(ht, &cur, (void**)&ktmp, (void**)&vtmp) == LSUP_OK) {
+        LSUP_buffer_done(vtmp);
+    }
+
+    printf("Freeing hash table.\n");
+    LSUP_htable_free(ht);
+
+    return 0;
+}
+
+
+int htable_tests()
+{
+    RUN(htable_idx);
+    RUN(htable_keys);
+
+    return 0;
+}