On 30/11/2022 09:53, Ævar Arnfjörð Bjarmason wrote:
On Tue, Nov 29 2022, Glen Choo wrote:Calvin Wan <calvinwan@xxxxxxxxxx> writes: So this might be more idiomatically written as: int n = strbuf_read_once(&pp->children[i].err, pp->children[i].process.err, 0); + if (opts->duplicate_output) { + struct strbuf buf = STRBUF_INIT; + strbuf_addbuf(&buf, &pp->children[i].err); + opts->duplicate_output(&buf, &pp->children[i].err, + opts->data, + pp->children[i].data); + strbuf_release(&buf); + } which also has the nice benefit of not touching the strbuf_read_once() line.We should also use "size_t n" there, not "int n", which is what it returns.
It returns an ssize_t not size_t, lower down we test `n < 0` so we certainly should not be using an unsigned type.
Best Wishes Phillip
