Fix several nasty bugs with bool ops.

include/buffer.h

@@ -151,11 +151,16 @@ LSUP_buffer_new_borrowed (unsigned char *data, const size_t size)
 
 
 /** @brief Free the content of a buffer.
+ *
+ * @note This function is safe to use on borrowed buffers.
  */
 void LSUP_buffer_done (LSUP_Buffer *buf);
 
 
 /** @brief Free a buffer.
+ *
+ * @note This function is safe to use on borrowed buffers. In such case, only
+ *  the `LSUP_Buffer` structure is freed, but not the underlying data.
  */
 void LSUP_buffer_free (LSUP_Buffer *buf);
 

include/namespace.h

@@ -20,6 +20,8 @@
  *
  * It contains a double hash map of pfx->ns and ns->pfx for fast 2-way lookup.
  *
+ * @FIXME that's not true, currently ns lookup by prefix involves an iteration.
+ *
  * Prefixes are fixed PFX_LEN-size strings, namespaces are arbitrary sized
  * strings.
  */

include/store_interface.h

@@ -156,20 +156,20 @@ typedef LSUP_rc (*store_txn_begin_fn_t)(void *store, int flags, void **txn);
  *
  * Only for LSUP_STORE_TXN stores.
  *
- * @param[in] store Store handle.
+ * @param[in] txn Transaction handle initialized by #store_txn_begin_fn_t().
  *
  * @return LSUP_OK if the transaction was committed successfully, <0 on error.
  */
-typedef LSUP_rc (*store_txn_commit_fn_t)(void *store);
+typedef LSUP_rc (*store_txn_commit_fn_t)(void *txn);
 
 
 /** @brief Abort a transaction.
  *
  * Only for LSUP_STORE_TXN stores.
  *
- * @param[in] store Store handle.
+ * @param[in] txn Transaction handle initialized by #store_txn_begin_fn_t().
  */
-typedef void (*store_txn_abort_fn_t)(void *store);
+typedef void (*store_txn_abort_fn_t)(void *txn);
 
 
 /** @brief Update the context of triples in a context-aware store.
@@ -402,17 +402,18 @@ typedef LSUP_NSMap * (*store_nsm_get_fn_t)(void *store);
 
 /** @brief Prototype: yield the matching triples and advance the iterator.
  *
- * NOTE: Iterators keep transactions open. Don't hold on to them longer than
+ * @note Iterators keep transactions open. Don't hold on to them longer than
  * necessary.
  *
- * NOTE: If the store interface has the LSUP_STORE_COW feature, the memory
+ * @note If the store interface has the LSUP_STORE_COW feature, the memory
  * buffer referenced by the #LSUP_Buffer handle is owned by the database. It
  * must not be written to or freed. To modify the data or use them beyond the
  * caller's scope, this memory must be copied. Note that the #LSUP_Buffer
  * handle must still be freed (with a plain `free()`), but not the underlying
- * data buffer, since only the latter is owned by the back end. For stores
- * without the LSUP_STORE_COW, data are copied on retrieval and the resulting
- * buffers can be freed with #LSUP_buffer_free() or analogous methods.
+ * data buffer, since only the latter is owned by the back end. Implementations
+ * with the `LSUP_STORE_COW` feature SHOULD create buffers with the
+ * `LSUP_BUF_BORROWED` flag, so that they can be safely freed with
+ * #LSUP_buffer_free() and #LSUP_btriple_free().
  *
  * @param[in] it Opaque iterator handle obtained with #store_lookup_fn_t.
  *

include/term.h

@@ -271,7 +271,8 @@ LSUP_lt_literal_new (const char *data, char *lang)
  *
  * Must be freed with #LSUP_term_free.
  *
- * @param[in] data The BNode identifier.
+ * @param[in] data The BNode identifier. It can be NULL, in which case, a
+ *  random identifier is minted.
  *
  * @return same as #LSUP_term_new().
  */

src/graph.c

@@ -77,8 +77,8 @@ LSUP_graph_get_txn (
 
     size_t _ct = 0;
     while (store->sif->lu_next_fn (it, sspo, NULL) == LSUP_OK) {
-        // TODO This is inefficient, it's deserializing a buffer triple that
-        // will be re-serialized by LSUP_graph_add_iter.
+        // This is deserializing a buffer triple that will be re-serialized by
+        // LSUP_graph_add_iter. But it's necessary to relativize URIs.
         LSUP_Triple *spo = LSUP_triple_new_from_btriple (sspo);
         LSUP_graph_add_iter (add_it, spo);
         LSUP_triple_free (spo);
@@ -141,24 +141,48 @@ LSUP_graph_bool_op_txn (
     // Handle transactions for graphs possibly in different storage back ends.
     void
         *lu1_txn = NULL,
-        *lu2_txn = NULL;
+        *lu2_txn = NULL,
+        *res_txn = NULL;
+    // Whether gr1 or gr2 txn will be open independently from res txn.
+    bool
+        open_txn1 = false,
+        open_txn2 = false;
 
     add_it = res->store->sif->add_init_fn (res->store->data, res_sc, txn);
 
-    // If either source graph is in the same store as the destination and has
-    // an open transaction, reuse that transaction. A new reader cannot be
-    // opened in LMDB while a writer is open already.
-    if (
-            gr1->store->sif->features & LSUP_STORE_TXN
-            && gr1->store == res->store)
-        lu1_txn = res->store->sif->iter_txn_fn (add_it);
+    if (res->store->sif->features & LSUP_STORE_TXN)
+        res_txn = res->store->sif->iter_txn_fn (add_it);
+
+    /* If either source graph is in the same store as the destination and has
+     * an open transaction, reuse that transaction. A new reader cannot be
+     * opened in LMDB while a writer is open already.
+     */
+    // Handle gr1 transaction.
     if (gr1->store->sif->features & LSUP_STORE_TXN) {
-        if (gr1->store == res->store)
-            lu2_txn = res->store->sif->iter_txn_fn (add_it);
-        else if (gr2->store == gr1->store)
-            lu2_txn = lu1_txn;
+        if (gr1->store == res->store) lu1_txn = res_txn;
+        // FIXME: MDB_RDONLY is implementation-specific and doesn't belong here.
+        else {
+            CHECK (gr1->store->sif->txn_begin_fn (
+                gr1->store->data, MDB_RDONLY, &lu1_txn), fail);
+            open_txn1 = true;
+        }
     }
 
+    // Handle gr2 transaction.
+    if (gr2->store->sif->features & LSUP_STORE_TXN) {
+        if (gr2->store == res->store) lu2_txn = res_txn;
+        else if (gr2->store == gr1->store) lu2_txn = lu1_txn;
+        // FIXME: see above.
+        else {
+            CHECK (gr2->store->sif->txn_begin_fn (
+                gr2->store->data, MDB_RDONLY, &lu2_txn), fail);
+            open_txn2 = true;
+        }
+    }
+    LOG_TRACE (
+            "lu1_txn: %p ; lu2_txn: %p ; res_txn: %p",
+            lu1_txn, lu2_txn, res_txn);
+
     /* BEGIN XOR block. */
 
     if (op == LSUP_BOOL_XOR) {
@@ -201,6 +225,9 @@ LSUP_graph_bool_op_txn (
     }
     gr1->store->sif->lu_free_fn (lu1_it);
 
+    if (open_txn1) gr1->store->sif->txn_commit_fn (lu1_txn);
+    if (open_txn2) gr2->store->sif->txn_commit_fn (lu2_txn);
+
     res->store->sif->add_done_fn (add_it);
     LSUP_btriple_free (sspo);
     LSUP_buffer_free (res_sc);
@@ -212,6 +239,8 @@ LSUP_graph_bool_op_txn (
     return rc;
 
 fail:
+    if (lu1_txn) gr1->store->sif->txn_abort_fn (lu1_txn);
+    if (lu2_txn) gr2->store->sif->txn_abort_fn (lu2_txn);
     LSUP_graph_free (res);
     return rc;
 }
@@ -626,7 +655,10 @@ LSUP_graph_print (LSUP_Graph *gr)
 {
     size_t ct;
     LSUP_GraphIterator *it = LSUP_graph_lookup (gr, NULL, NULL, NULL, &ct);
-    if (UNLIKELY (!it)) log_error ("Could not print graph contents.");
+    if (UNLIKELY (!it)) {
+        log_error ("Could not inspect graph for printing.");
+        return;
+    }
 
     printf ("*** Graph %s (%zu triples):\n\n", gr->uri->data, ct);
 

src/store_htable.c

@@ -468,6 +468,7 @@ tkey_to_strp (
         if (UNLIKELY (!tmp)) return LSUP_DB_ERR;
         LSUP_btriple_pos(sspo, i)->addr = tmp->data;
         LSUP_btriple_pos(sspo, i)->size = tmp->size;
+        LSUP_btriple_pos(sspo, i)->flags |= LSUP_BUF_BORROWED;
     }
 
     return LSUP_OK;

src/store_mdb.c

@@ -742,7 +742,7 @@ mdbstore_lookup (
     else {
         it->rc = mdb_txn_begin (it->store->env, NULL, MDB_RDONLY, &it->txn);
         if (it->rc != MDB_SUCCESS) {
-            log_error ("Database error: %s", LSUP_strerror (it->rc));
+            log_error ("Database error in lookup: %s", LSUP_strerror (it->rc));
             return NULL;
         }
         LOG_TRACE ("Opening new MDB transaction @%p", it->txn);

test/test_graph.c

@@ -126,6 +126,8 @@ _graph_get (LSUP_StoreType type)
     EXPECT_INT_EQ (LSUP_graph_size (gr3), LSUP_graph_size (gr1));
     EXPECT_INT_EQ (LSUP_graph_size (gr4), LSUP_graph_size (gr2));
 
+    ASSERT (!LSUP_graph_equals (gr1, gr2), "Graphs 1 and 2 are equal!");
+    ASSERT (!LSUP_graph_equals (gr3, gr4), "Graphs 3 and 4 are equal!");
     ASSERT (LSUP_graph_equals (gr1, gr3), "Graphs 1 and 3 are not equal!");
     ASSERT (LSUP_graph_equals (gr2, gr4), "Graphs 2 and 4 are not equal!");