On Fri, Oct 28 2022, Patrick Steinhardt wrote:
> @@ -125,6 +125,13 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
>
> rev_list_in = xfdopen(rev_list.in, "w");
>
> + if (opt->reachable_oids_fn) {
> + const struct object_id *reachable_oid;
> + while ((reachable_oid = opt->reachable_oids_fn(opt->reachable_oids_data)) != NULL)
> + if (fprintf(rev_list_in, "^%s\n", oid_to_hex(reachable_oid)) < 0)
> + break;
Just a style nit, we tend to avoid != NULL, != 0 etc. comparisons. I see
connected.c has some of that already, but for new code let's just check
truthiness.
Also for such a small scope a shorter variable name helps us stay at the
usual column limits:
if (opt->reachable_oids_fn) {
const struct object_id *oid;
while ((oid = opt->reachable_oids_fn(opt->reachable_oids_data)))
if (fprintf(rev_list_in, "^%s\n", oid_to_hex(oid)) < 0)
break;
The fprintf() return value checking seemed a bit odd, not because we
shouldn't do it, but because we usually don't bother. For other
reviewers: We have that form already in connected.c, so at least locally
we're not being diligently careful, only to have it undone by adjacent
code...
Looks good!
