On Tue, 3 Apr 2018 12:51:42 -0400 Derrick Stolee <dstolee@xxxxxxxxxxxxx> wrote: > -static int queue_has_nonstale(struct prio_queue *queue) > +static int queue_has_nonstale(struct prio_queue *queue, uint32_t min_gen) > { > - int i; > - for (i = 0; i < queue->nr; i++) { > - struct commit *commit = queue->array[i].data; > - if (!(commit->object.flags & STALE)) > - return 1; > + if (min_gen != GENERATION_NUMBER_UNDEF) { > + if (queue->nr > 0) { > + struct commit *commit = queue->array[0].data; > + return commit->generation >= min_gen; > + } This only works if the prio_queue has compare_commits_by_gen_then_commit_date. Also, I don't think that the min_gen != GENERATION_NUMBER_UNDEF check is necessary. So I would write this as: if (queue->compare == compare_commits_by_gen_then_commit_date && queue->nr) { struct commit *commit = queue->array[0].data; return commit->generation >= min_gen; } for (i = 0 ... If you'd rather not perform the comparison to compare_commits_by_gen_then_commit_date every time you invoke queue_has_nonstale(), that's fine with me too, but document somewhere that queue_has_nonstale() only works if this comparison function is used. > + if (commit->generation > last_gen) > + BUG("bad generation skip"); > + > + last_gen = commit->generation; last_gen seems to only be used to ensure that the priority queue returns elements in the correct order - I think we can generally trust the queue, and if we need to test it, we can do it elsewhere.