Allows us to have more elements in the heap, and fixes an issue where the element inserted at index 0 may have been counted as not inserted.
#include <freeradius-devel/io/control.h>
#include <freeradius-devel/io/message.h>
#include <freeradius-devel/util/dlist.h>
+#include <freeradius-devel/util/heap.h>
#include <freeradius-devel/util/log.h>
#include <sys/types.h>
*/
struct {
fr_channel_t *ch; //!< channel where this messages was received
- int32_t heap_id; //!< for the various queues
+ fr_heap_index_t heap_id; //!< for the various queues
} channel;
};
*
*/
typedef struct {
- int32_t heap_id; //!< workers are in a heap
+ fr_heap_index_t heap_id; //!< workers are in a heap
fr_time_t cpu_time; //!< how much CPU time this worker has spent
fr_time_t predicted; //!< predicted processing time for one packet
},
.channel = {
- .heap_id = -1,
+ .heap_id = 0,
},
.listen = li,
/*
* Ensure that heap insert works.
*/
- cd->channel.heap_id = -1;
+ cd->channel.heap_id = 0;
if (fr_heap_insert(nr->replies, cd) < 0) {
fr_message_done(&cd->m);
fr_assert(0 == 1);
fr_time_tracking_yield(&request->async->tracking, now);
worker->num_active++;
- fr_assert(request->runnable_id < 0);
+ fr_assert(!fr_heap_entry_inserted(request->runnable_id));
(void) fr_heap_insert(worker->runnable, request);
if (!worker->ev_cleanup) worker_max_request_timer(worker);
/*
* If we're sending a reply, then it's no longer runnable.
*/
- fr_assert(request->runnable_id < 0);
+ fr_assert(!fr_heap_entry_inserted(request->runnable_id));
if (!size) {
size = request->async->listen->app_io->default_reply_size;
((request = fr_heap_pop(worker->runnable)) != NULL)) {
REQUEST_VERIFY(request);
- fr_assert(request->runnable_id < 0);
+ fr_assert(!fr_heap_entry_inserted(request->runnable_id));
/*
* For real requests, if the channel is gone,
}
typedef struct cf_file_heap_t {
- char const *filename;
- int heap_id;
+ char const *filename;
+ fr_heap_index_t heap_id;
} cf_file_heap_t;
static int8_t filename_cmp(void const *one, void const *two)
MEM(h = talloc_zero(frame->heap, cf_file_heap_t));
MEM(h->filename = talloc_typed_strdup(h, stack->buff[1]));
- h->heap_id = -1;
+ h->heap_id = 0;
(void) fr_heap_insert(frame->heap, h);
}
closedir(dir);
struct fr_pool_connection_s {
fr_pool_connection_t *prev; //!< Previous connection in list.
fr_pool_connection_t *next; //!< Next connection in list.
- int32_t heap_id; //!< For the next connection heap.
+ fr_heap_index_t heap_id; //!< For the next connection heap.
time_t created; //!< Time connection was created.
fr_time_t last_reserved; //!< Last time the connection was reserved.
.flags = {
.detachable = args->detachable
},
- .runnable_id = -1,
- .time_order_id = -1,
-
.alloc_file = file,
.alloc_line = line
};
* if the request is freed out of
* the free list.
*/
- request->time_order_id = -1;
- request->runnable_id = -1;
+ request->time_order_id = 0;
+ request->runnable_id = 0;
#endif
/*
rlm_rcode_t rcode; //!< Last rcode returned by a module
fr_rb_node_t dedup_node; //!< entry in the deduplication tree.
- int32_t runnable_id; //!< entry in the heap of runnable packets
- int32_t time_order_id; //!< entry in the heap of time ordered packets
+ fr_heap_index_t runnable_id; //!< entry in the heap of runnable packets
+ fr_heap_index_t time_order_id; //!< entry in the heap of time ordered packets
uint32_t options; //!< mainly for proxying EAP-MSCHAPv2.
uint64_t id; //!< Trunk request ID.
- int32_t heap_id; //!< Used to track the request conn->pending heap.
+ fr_heap_index_t heap_id; //!< Used to track the request conn->pending heap.
fr_dlist_t entry; //!< Used to track the trunk request in the conn->sent
///< or trunk->backlog request.
///< This *MUST* be the first field in this
///< structure.
- int32_t heap_id; //!< Used to track the connection in the connected
+ fr_heap_index_t heap_id; //!< Used to track the connection in the connected
///< heap.
fr_dlist_t entry; //!< Used to track the connection in the connecting,
MEM(tconn = talloc_zero(trunk, fr_trunk_connection_t));
tconn->pub.trunk = trunk;
tconn->pub.state = FR_TRUNK_CONN_HALTED; /* All connections start in the halted state */
- tconn->heap_id = -1; /* Helps with asserts */
/*
* Allocate a new fr_connection_t or fail.
fr_event_timer_t const **parent; //!< A pointer to the parent structure containing the timer
///< event.
- int32_t heap_id; //!< Where to store opaque heap data.
+ fr_heap_index_t heap_id; //!< Where to store opaque heap data.
fr_dlist_t entry; //!< List of deferred timer events.
fr_event_list_t *el; //!< Event list containing this timer.
void *uctx; //!< Context pointer to pass to each file descriptor callback.
#ifdef LOCAL_PID
- int32_t heap_id;
+ fr_heap_index_t heap_id;
#endif
#ifndef NDEBUG
if (ctx != el) talloc_link_ctx(ctx, ev);
talloc_set_destructor(ev, _event_timer_free);
- ev->heap_id = -1;
+ ev->heap_id = 0;
} else {
memcpy(&ev, ev_p, sizeof(ev)); /* Not const to us */
}
#else /* LOCAL_PID */
- ev->heap_id = -1;
+ ev->heap_id = 0;
if (unlikely(fr_heap_insert(el->pids, ev) < 0)) {
fr_strerror_printf("Failed adding waiter for PID %ld - %s", (long) pid, fr_strerror());
talloc_free(ev);
*/
struct fr_heap_s {
- size_t size; //!< Number of nodes allocated.
+ unsigned int size; //!< Number of nodes allocated.
size_t offset; //!< Offset of heap index in element structure.
- int32_t num_elements; //!< Number of nodes used.
+ unsigned int num_elements; //!< Number of nodes used.
- char const *type; //!< Type of elements.
+ char const *type; //!< Talloc type of elements.
fr_heap_cmp_t cmp; //!< Comparator function.
void **p; //!< Array of nodes.
};
/*
- * First node in a heap is element 0. Children of i are 2i+1 and
- * 2i+2. These macros wrap the logic, so the code is more
+ * First node in a heap is element 1. Children of i are 2i and
+ * 2i+1. These macros wrap the logic, so the code is more
* descriptive.
*/
-#define HEAP_PARENT(_x) (((_x) - 1 ) / 2)
-#define HEAP_LEFT(_x) (2 * (_x) + 1)
-/* #define HEAP_RIGHT(_x) (2 * (_x) + 2 ) */
+#define HEAP_PARENT(_x) ((_x) >> 1)
+#define HEAP_LEFT(_x) (2 * (_x))
+/* #define HEAP_RIGHT(_x) (2 * (_x) + 1 ) */
#define HEAP_SWAP(_a, _b) { void *_tmp = _a; _a = _b; _b = _tmp; }
-static void fr_heap_bubble(fr_heap_t *hp, int32_t child);
+static void fr_heap_bubble(fr_heap_t *hp, fr_heap_index_t child);
fr_heap_t *_fr_heap_alloc(TALLOC_CTX *ctx, fr_heap_cmp_t cmp, char const *type, size_t offset)
{
- fr_heap_t *fh;
+ fr_heap_t *hp;
- fh = talloc_zero(ctx, fr_heap_t);
- if (!fh) return NULL;
+ hp = talloc_zero(ctx, fr_heap_t);
+ if (!hp) return NULL;
- fh->size = 2048;
- fh->p = talloc_array(fh, void *, fh->size);
- if (!fh->p) {
- talloc_free(fh);
+ hp->size = 2048;
+ hp->p = talloc_array(hp, void *, hp->size);
+ if (!hp->p) {
+ talloc_free(hp);
return NULL;
}
- fh->type = type;
- fh->cmp = cmp;
- fh->offset = offset;
+ /*
+ * As we're using unsigned index values
+ * index 0 is a special value meaning
+ * that the data isn't currently inserted
+ * into the heap.
+ */
+ hp->p[0] = UINTPTR_MAX;
+
+ hp->type = type;
+ hp->cmp = cmp;
+ hp->offset = offset;
- return fh;
+ return hp;
}
-static inline CC_HINT(always_inline) CC_HINT(nonnull) int32_t index_get(fr_heap_t *hp, void *data)
+static inline CC_HINT(always_inline, nonnull) fr_heap_index_t index_get(fr_heap_t *hp, void *data)
{
- return *((int32_t const *)(((uint8_t const *)data) + hp->offset));
+ return *((fr_heap_index_t const *)(((uint8_t const *)data) + hp->offset));
}
-static inline CC_HINT(always_inline) CC_HINT(nonnull) void index_set(fr_heap_t *hp, void *data, int32_t idx)
+static inline CC_HINT(always_inline, nonnull) void index_set(fr_heap_t *hp, void *data, fr_heap_index_t idx)
{
- *((int32_t *)(((uint8_t *)data) + hp->offset)) = idx;
+ *((fr_heap_index_t *)(((uint8_t *)data) + hp->offset)) = idx;
}
#define OFFSET_SET(_heap, _idx) index_set(_heap, _heap->p[_idx], _idx);
-#define OFFSET_RESET(_heap, _idx) index_set(_heap, _heap->p[_idx], -1);
+#define OFFSET_RESET(_heap, _idx) index_set(_heap, _heap->p[_idx], 0);
/** Insert a new element into the heap
*
*/
int fr_heap_insert(fr_heap_t *hp, void *data)
{
- int32_t child;
+ fr_heap_index_t child;
- /*
- * On insert, the heap_id MUST be either:
- *
- * -1 = the node was added / removed from the heap
- * and the heap code set the ID to -1
- * 0 = the node was just allocated via an "alloc_zero"
- * function
- */
child = index_get(hp, data);
- if ((child > 0) || ((child == 0) && (hp->num_elements > 0) && (data == hp->p[0]))) {
+ if (fr_heap_entry_inserted(child)) {
fr_strerror_const("Node is already in the heap");
return -1;
}
- child = hp->num_elements;
+ child = hp->num_elements + 1; /* Avoid using index 0 */
#ifndef TALLOC_GET_TYPE_ABORT_NOOP
if (hp->type) (void)_talloc_get_type_abort(data, hp->type, __location__);
/*
* Heap is full. Double it's size.
*/
- if ((size_t)child == hp->size) {
- void **n;
- size_t n_size = hp->size * 2;
+ if (child == hp->size) {
+ void **n;
+ unsigned int n_size;
/*
- * heap_id is a 32-bit signed integer. If the heap will
- * grow to contain more than 2B elements, disallow
+ * heap_id is a 32-bit unsigned integer. If the heap will
+ * grow to contain more than 4B elements, disallow
* integer overflow. Tho TBH, that should really never
* happen.
*/
- if (n_size > INT32_MAX) {
- if (hp->size == INT32_MAX) {
+ if (hp->size > (UINT_MAX - hp->size)) {
+ if (hp->size == UINT_MAX) {
fr_strerror_const("Heap is full");
return -1;
} else {
- n_size = INT32_MAX;
+ n_size = UINT_MAX;
}
+ } else {
+ n_size = hp->size * 2;
}
n = talloc_realloc(hp, hp->p, void *, n_size);
if (!n) {
- fr_strerror_printf("Failed expanding heap to %zu elements (%zu bytes)",
- n_size, (n_size * sizeof(void *)));
+ fr_strerror_printf("Failed expanding heap to %u elements (%u bytes)",
+ n_size, (n_size * (unsigned int)sizeof(void *)));
return -1;
}
hp->size = n_size;
return 0;
}
-static void fr_heap_bubble(fr_heap_t *hp, int32_t child)
+static inline void fr_heap_bubble(fr_heap_t *hp, fr_heap_index_t child)
{
+ if (!fr_cond_assert(child > 0)) return;
+
/*
* Bubble up the element.
*/
- while (child > 0) {
- int32_t parent = HEAP_PARENT(child);
+ while (child > 1) {
+ fr_heap_index_t parent = HEAP_PARENT(child);
/*
* Parent is smaller than the child. We're done.
OFFSET_SET(hp, child);
}
-
/** Remove a node from the heap
*
* @param[in] hp The heap to extract an element from.
*/
int fr_heap_extract(fr_heap_t *hp, void *data)
{
- int32_t parent, child, max;
+ fr_heap_index_t parent, child, max;
/*
* Extract element.
/*
* Out of bounds.
*/
- if (unlikely((parent < 0) || (parent >= hp->num_elements))) {
+ if (unlikely((parent == 0) || (parent > hp->num_elements))) {
fr_strerror_printf("Heap parent (%i) out of bounds (0-%i)", parent, hp->num_elements);
return -1;
}
parent, hp->p[parent]);
return -1;
}
- max = hp->num_elements - 1;
+ max = hp->num_elements;
- OFFSET_RESET(hp, parent);
child = HEAP_LEFT(parent);
+ OFFSET_RESET(hp, parent);
while (child <= max) {
/*
* Maybe take the right child.
return 0;
}
-
void *fr_heap_peek(fr_heap_t *hp)
{
- if (!hp || (hp->num_elements == 0)) return NULL;
+ if (hp->num_elements == 0) return NULL;
- return hp->p[0];
+ return hp->p[1];
}
void *fr_heap_pop(fr_heap_t *hp)
if (hp->num_elements == 0) return NULL;
- data = hp->p[0];
- (void) fr_heap_extract(hp, data);
+ data = hp->p[1];
+ if (unlikely(fr_heap_extract(hp, data) < 0)) return NULL;
return data;
}
-
void *fr_heap_peek_tail(fr_heap_t *hp)
{
- if (!hp || (hp->num_elements == 0)) return NULL;
+ if (hp->num_elements == 0) return NULL;
/*
* If this is NULL, we have a problem.
*/
- return hp->p[hp->num_elements - 1];
+ return hp->p[hp->num_elements];
}
-uint32_t fr_heap_num_elements(fr_heap_t *hp)
+/** Return the number of elements in the heap
+ *
+ * @param[in] hp to return the number of elements from.
+ */
+unsigned int fr_heap_num_elements(fr_heap_t *hp)
{
- if (!hp) return 0;
-
- return (uint32_t)hp->num_elements;
+ return hp->num_elements;
}
/** Iterate over entries in heap
*/
void *fr_heap_iter_init(fr_heap_t *hp, fr_heap_iter_t *iter)
{
- *iter = 0;
+ *iter = 1;
- if (unlikely(!hp) || (hp->num_elements == 0)) return NULL;
+ if (hp->num_elements == 0) return NULL;
- return hp->p[0];
+ return hp->p[1];
}
/** Get the next entry in a heap
*/
void *fr_heap_iter_next(fr_heap_t *hp, fr_heap_iter_t *iter)
{
- if (unlikely(!hp)) return NULL;
-
- if ((*iter + 1) >= hp->num_elements) return NULL;
+ if ((*iter + 1) > hp->num_elements) return NULL;
*iter += 1;
return hp->p[*iter];
#include <stdint.h>
#include <sys/types.h>
-typedef int32_t fr_heap_iter_t;
+typedef unsigned int fr_heap_index_t;
+typedef unsigned int fr_heap_iter_t;
/*
* Return negative numbers to put 'a' at the top of the heap.
/** Check if an entry is inserted into a heap
*
*/
-static inline bool fr_heap_entry_inserted(int32_t heap_id)
+static inline bool fr_heap_entry_inserted(fr_heap_index_t heap_idx)
{
- return (heap_id >= 0);
+ return (heap_idx > 0);
}
int fr_heap_insert(fr_heap_t *hp, void *data) CC_HINT(nonnull);
int fr_heap_extract(fr_heap_t *hp, void *data) CC_HINT(nonnull);
void *fr_heap_pop(fr_heap_t *hp) CC_HINT(nonnull);
-void *fr_heap_peek(fr_heap_t *hp);
-void *fr_heap_peek_tail(fr_heap_t *hp);
+void *fr_heap_peek(fr_heap_t *hp) CC_HINT(nonnull);
+void *fr_heap_peek_tail(fr_heap_t *hp) CC_HINT(nonnull);
-uint32_t fr_heap_num_elements(fr_heap_t *hp);
+uint32_t fr_heap_num_elements(fr_heap_t *hp) CC_HINT(nonnull);
-void *fr_heap_iter_init(fr_heap_t *hp, fr_heap_iter_t *iter);
-void *fr_heap_iter_next(fr_heap_t *hp, fr_heap_iter_t *iter);
+void *fr_heap_iter_init(fr_heap_t *hp, fr_heap_iter_t *iter) CC_HINT(nonnull);
+void *fr_heap_iter_next(fr_heap_t *hp, fr_heap_iter_t *iter) CC_HINT(nonnull);
#ifdef __cplusplus
}
static bool fr_heap_check(fr_heap_t *hp, void *data)
{
- int i;
+ unsigned int i;
if (!hp || (hp->num_elements == 0)) return false;
for (i = 0; i < hp->num_elements; i++) {
- if (hp->p[i] == data) {
+ if (hp->p[i + 1] == data) {
return true;
}
}
}
typedef struct {
- int data;
- int32_t heap; /* for the heap */
+ int data;
+ unsigned int heap; /* for the heap */
} heap_thing;
TEST_CASE("deletions");
{
- int32_t entry;
+ unsigned int entry;
for (i = 0; i < HEAP_TEST_SIZE / skip; i++) {
entry = i * skip;
- TEST_CHECK(array[entry].heap != -1);
+ TEST_CHECK(array[entry].heap != 0);
TEST_MSG("element %i removed out of order", entry);
TEST_CHECK((ret = fr_heap_extract(hp, &array[entry])) >= 0);
- TEST_MSG("element %i removal failed, returned %i", entry, ret);
+ TEST_MSG("element %i removal failed, returned %i - %s", entry, ret, fr_strerror());
TEST_CHECK(!fr_heap_check(hp, &array[entry]));
TEST_MSG("element %i removed but still in heap", entry);
- TEST_CHECK(array[entry].heap == -1);
+ TEST_CHECK(array[entry].heap == 0);
TEST_MSG("element %i removed out of order", entry);
}
}
fr_heap_t *hp;
int i;
heap_thing *array;
- heap_thing *thing;
+ heap_thing *thing, *prev = NULL;
int data = 0;
unsigned int count = 0;
int ret;
while ((thing = fr_heap_pop(hp))) {
TEST_CHECK(thing->data >= data);
- data = thing->data;
+ TEST_MSG("Expected data >= %i, got %i", data, thing->data);
+ if (thing->data >= data) data = thing->data;
+ TEST_CHECK(thing != prev);
+ prev = thing;
count++;
}
TEST_MSG("expected %i elements remaining in the heap", to_remove - i);
TEST_CHECK(fr_heap_extract(hp, t) >= 0);
- TEST_MSG("failed extracting %i", i);
+ TEST_MSG("failed extracting %i - %s", i, fr_strerror());
}
/*
removed = 0;
for (i = 0; i < HEAP_CYCLE_SIZE; i++) {
- if (array[i].heap == -1) {
+ if (!fr_heap_entry_inserted(array[i].heap)) {
TEST_CHECK((ret = fr_heap_insert(hp, &array[i])) >= 0);
TEST_MSG("insert failed, returned %i - %s", ret, fr_strerror());
inserted++;
} else {
TEST_CHECK((ret = fr_heap_extract(hp, &array[i])) >= 0);
- TEST_MSG("element %i removal failed, returned %i", i, ret);
+ TEST_MSG("element %i removal failed, returned %i - %s", i, ret, fr_strerror());
removed++;
}
}
*
* @param[in] ft to return node count for.
*/
-uint32_t fr_trie_num_elements(UNUSED fr_trie_t *ft)
+unsigned int fr_trie_num_elements(UNUSED fr_trie_t *ft)
{
return 0;
}
bool fr_trie_delete(fr_trie_t *ft, void const *data) CC_HINT(nonnull);
-uint32_t fr_trie_num_elements(fr_trie_t *ft) CC_HINT(nonnull); /* always returns 0 */
+unsigned int fr_trie_num_elements(fr_trie_t *ft) CC_HINT(nonnull); /* always returns 0 */
#ifdef __cplusplus
}
rlm_cache_entry_t fields; //!< Entry data.
fr_rb_node_t node; //!< Entry used for lookups.
- int32_t heap_id; //!< Offset used for expiry heap.
+ fr_heap_index_t heap_id; //!< Offset used for expiry heap.
} rlm_cache_rb_entry_t;
/** Compare two entries by key
RERROR("Failed allocating cache entry");
return NULL;
}
- c->heap_id = -1;
return (rlm_cache_entry_t *)c;
}
projects / thirdparty / freeradius-server.git / commitdiff
