From: Stefan Sperling Subject: Re: Remove got_pathlist_append() To: Kyle Ackerman Cc: "gameoftrees@openbsd.org" Date: Thu, 21 Nov 2024 09:08:11 +0100 On Thu, Nov 21, 2024 at 01:13:12AM +0000, Kyle Ackerman wrote: > @@ -780,11 +781,12 @@ got_privsep_recv_fetch_progress(int *done, struct got_ > } > off += s->target_len; > remain -= s->target_len; > - err = got_pathlist_append(symrefs, name, target); > + err = got_pathlist_insert(&new, symrefs, name, target); > if (err) { Missing new == NULL condition here? > free(name); > free(target); > - goto done; > + if (err->code != GOT_ERR_REF_DUP_ENTRY) > + goto done; > } > } > break; > @@ -938,6 +939,7 @@ main(int argc, char **argv) > struct got_object_id *id; > char *refname; > > + > err = got_privsep_recv_imsg(&imsg, &ibuf, 0); > if (err) { > if (err->code == GOT_ERR_PRIVSEP_PIPE) Needless whitespace change above. > --- regress/cmdline/diff.sh > +++ regress/cmdline/diff.sh > @@ -1941,8 +1941,8 @@ date: $d > > changed epsilon into file > > - D epsilon/zeta > A epsilon > + D epsilon/zeta > The above chunk of the patch did not apply with 'got patch', probably because leading spaces are present on these lines. Not sure why, and not a deal-breaker of course. > Thoughts/comments/suggestions? I'm ok with this. Would you mind me addressing the above nits before comitting your diff? As I recall, the reason for having the _append function was to allow for keeping paths sorted in the order given on the command line, such that the output of a command would reflect the order in which paths were specified by the user. However, this was never used effectively, and at least in the case of 'got diff' it was later decided to sort specified paths to provide consistent output regardless of how the path arguments are provided. Today, I cannot think of a reason why paths shouldn't always be sorted, and with an RB tree we cannot have it any other way.