On Thu, Jun 06, 2024 at 08:53:37PM +0530, Chandra Pratap wrote: > According to Documentation/CodingGuidelines, control-flow statements > with a single line as their body must omit curly braces. Make > reftable/pq.c conform to this guideline. Besides that, remove > unnecessary newlines and variable assignment. > > Mentored-by: Patrick Steinhardt <ps@xxxxxx> > Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> > Signed-off-by: Chandra Pratap <chandrapratap3519@xxxxxxxxx> > --- > reftable/pq.c | 18 ++++-------------- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/reftable/pq.c b/reftable/pq.c > index 7fb45d8c60..0401c47068 100644 > --- a/reftable/pq.c > +++ b/reftable/pq.c > @@ -27,22 +27,16 @@ struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq) > pq->heap[0] = pq->heap[pq->len - 1]; > pq->len--; > > - i = 0; > while (i < pq->len) { > int min = i; > int j = 2 * i + 1; > int k = 2 * i + 2; > - if (j < pq->len && pq_less(&pq->heap[j], &pq->heap[i])) { > + if (j < pq->len && pq_less(&pq->heap[j], &pq->heap[i])) > min = j; > - } > - if (k < pq->len && pq_less(&pq->heap[k], &pq->heap[min])) { > + if (k < pq->len && pq_less(&pq->heap[k], &pq->heap[min])) > min = k; > - } > - > - if (min == i) { > + if (min == i) > break; > - } > - > SWAP(pq->heap[i], pq->heap[min]); > i = min; > } > @@ -53,19 +47,15 @@ struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq) > void merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e) > { > int i = 0; > - Nit: I think this newline is helpful as it delimits the variable declarations from the actual code. I wonder whether we should also change the type of `i` to `size_t` while at it, as `pq->len` is of type `size_t, as well (and further up in the other test). It does mean that we have to add an explicit check whether `pq->len == 0`, but that isn't all that bad. In any case, if we want to do such a change, it should probably be in a separate commit. > REFTABLE_ALLOC_GROW(pq->heap, pq->len + 1, pq->cap); > pq->heap[pq->len++] = *e; > > i = pq->len - 1; > while (i > 0) { > int j = (i - 1) / 2; The type of `j` is wrong, as well. Other than that this patch series looks good to me, thanks. Patrick
Attachment:
signature.asc
Description: PGP signature