62e745ced2
The alloc and nr fields of a prio-queue tell us how much memory is
allocated and used in the array. So the natural type for them is size_t,
which prevents overflow on 64-bit systems where "int" is still 32 bits.
This is unlikely to happen in practice, as we typically use it for
storing commits, and having 2^31 of those is rather a lot. But it's good
to keep our generic data structures as flexible as possible. And as we
start to enforce -Wsign-compare, it means that callers need to use
"int", too, and the problem proliferates. Let's fix it at the source.
The changes here can be put into a few groups:
1. Changing the alloc/nr fields in the struct to size_t. This requires
swapping out int for size_t in negotiator/skipping.c, as well as
in prio_queue_get(), because those all iterate over the array.
Building with -Wsign-compare complains about these.
2. Other code that assigns or passes around indexes into the array
(e.g., the swap() and compare() functions) won't trigger
-Wsign-compare because we are simply truncating the values. These
are caught by -Wconversion, but I've adjusted them here to
future-proof us.
3. In prio_queue_reverse() we compute "queue->nr - 1" without checking
if anything is in the queue, which underflows now that nr is
unsigned. We can fix that by returning early when the queue is
empty (there is nothing to reverse).
4. The insertion_ctr variable is currently unsigned, but can likewise
grow (it is actually worse, because adding and removing an element
many times will keep increasing the counter, even though "nr" does
not). I've bumped that to size_t here, as well.
But -Wconversion notes that computing the "cmp" result by
subtracting the counters and assigning to "int" is a potential
problem. And that's true even before this patch, since we use an
unsigned counter (imagine comparing "2^32-1" and "0", which should
be a high positive value, but instead is "-1" as a signed int).
Since we only care about the sign (and not the magnitude) of the
result, we could fix this by swapping out the subtraction for a
ternary comparison. Probably the performance impact would be
negligible, since we just called into a custom compare function and
branched on its result anyway. But it's easy enough to do a
branchless version by subtracting the comparison results.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
61 lines
1.6 KiB
C
61 lines
1.6 KiB
C
#ifndef PRIO_QUEUE_H
|
|
#define PRIO_QUEUE_H
|
|
|
|
/*
|
|
* A priority queue implementation, primarily for keeping track of
|
|
* commits in the 'date-order' so that we process them from new to old
|
|
* as they are discovered, but can be used to hold any pointer to
|
|
* struct. The caller is responsible for supplying a function to
|
|
* compare two "things".
|
|
*
|
|
* Alternatively, this data structure can also be used as a LIFO stack
|
|
* by specifying NULL as the comparison function.
|
|
*/
|
|
|
|
/*
|
|
* Compare two "things", one and two; the third parameter is cb_data
|
|
* in the prio_queue structure. The result is returned as a sign of
|
|
* the return value, being the same as the sign of the result of
|
|
* subtracting "two" from "one" (i.e. negative if "one" sorts earlier
|
|
* than "two").
|
|
*/
|
|
typedef int (*prio_queue_compare_fn)(const void *one, const void *two, void *cb_data);
|
|
|
|
struct prio_queue_entry {
|
|
unsigned ctr;
|
|
void *data;
|
|
};
|
|
|
|
struct prio_queue {
|
|
prio_queue_compare_fn compare;
|
|
unsigned insertion_ctr;
|
|
void *cb_data;
|
|
size_t alloc, nr;
|
|
struct prio_queue_entry *array;
|
|
};
|
|
|
|
/*
|
|
* Add the "thing" to the queue.
|
|
*/
|
|
void prio_queue_put(struct prio_queue *, void *thing);
|
|
|
|
/*
|
|
* Extract the "thing" that compares the smallest out of the queue,
|
|
* or NULL. If compare function is NULL, the queue acts as a LIFO
|
|
* stack.
|
|
*/
|
|
void *prio_queue_get(struct prio_queue *);
|
|
|
|
/*
|
|
* Gain access to the "thing" that would be returned by
|
|
* prio_queue_get, but do not remove it from the queue.
|
|
*/
|
|
void *prio_queue_peek(struct prio_queue *);
|
|
|
|
void clear_prio_queue(struct prio_queue *);
|
|
|
|
/* Reverse the LIFO elements */
|
|
void prio_queue_reverse(struct prio_queue *);
|
|
|
|
#endif /* PRIO_QUEUE_H */
|