| Message ID | 20240606154712.15935-2-chandrapratap3519@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t: port reftable/pq_test.c to the unit testing | expand |
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@pks.im> > Mentored-by: Christian Couder <chriscool@tuxfamily.org> > Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> > --- > 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
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; - 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; - if (pq_less(&pq->heap[j], &pq->heap[i])) { + if (pq_less(&pq->heap[j], &pq->heap[i])) break; - } - SWAP(pq->heap[j], pq->heap[i]); - i = j; } }
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@pks.im> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com> --- reftable/pq.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)