glib-aux/prioq: assert for valid index in find_item() of NMPrioq

NMPrioq is taken from systemd's "prioq.c". It is a nice data structure,
that accepts and an index pointer, to directly access elements inside
the heap.

Previously, the API didn't require a consistent index, while the data is
not inside the heap. nm_prioq_{update,shuffle,remove}()) call find_item(),
which silently accepts wrong indexes and assumes the element is not in
the heap.

Keeping the index in sync with the data seems error prone. Accepting any
index without asserting may be convenient for the user (as the user is
not required to pre-initialize the index with NM_PRIOQ_IDX_NULL).
However, it also misses to catch potential bugs.

Now the index must be kept consistent, in particular also if the element
is not enqueued. This means, you must initialize them with
NM_PRIOQ_IDX_NULL.

2 files changed, 16 insertions, 1 deletions

diff --git a/src/libnm-glib-aux/nm-prioq.c b/src/libnm-glib-aux/nm-prioq.c
index c53e74fc1c..897d65dd51 100644
--- a/src/libnm-glib-aux/nm-prioq.c
+++ b/src/libnm-glib-aux/nm-prioq.c
@@ -288,14 +288,28 @@ find_item(NMPrioq *q, void *data, unsigned *idx)
         return NULL;
     }
 
+    /* If the user however provides an "idx" pointer, then we assert that it is
+     * consistent. That is, if data is not in the queue, then we require that
+     * "*idx" is NM_PRIOQ_IDX_NULL, and otherwise we require that we really
+     * find "data" at index "*idx".
+     *
+     * This means, when the user calls nm_prioq_{remove,update,reshuffle}()
+     * with an "idx", then they must make sure that the index is consistent.
+     * Usually this means they are required to initialize the index to
+     * NM_PRIOQ_IDX_NULL while the data is not in the heap.
+     *
+     * This is done to assert more, and requires a stricter usage of the API
+     * (in the hope to find misuses of the index). */
+
     if (*idx >= q->_priv.n_items) {
+        nm_assert(*idx == NM_PRIOQ_IDX_NULL);
         return NULL;
     }
 
     i = &q->_priv.items[*idx];
 
     if (i->data != data)
-        return NULL;
+        return nm_assert_unreachable_val(NULL);
 
     return i;
 }
diff --git a/src/libnm-lldp/nm-lldp-neighbor.c b/src/libnm-lldp/nm-lldp-neighbor.c
index f1e2d42eb0..a2a9695e85 100644
--- a/src/libnm-lldp/nm-lldp-neighbor.c
+++ b/src/libnm-lldp/nm-lldp-neighbor.c
@@ -735,6 +735,7 @@ nm_lldp_neighbor_new(size_t raw_size)
 
     n->raw_size  = raw_size;
     n->ref_count = 1;
+    n->prioq_idx = NM_PRIOQ_IDX_NULL;
     return n;
 }