On Fri, Feb 2, 2018 at 4:06 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: > On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren <newren@xxxxxxxxx> wrote: >> + ret = malloc(sizeof(struct diff_queue_struct)); > > Please use xmalloc() and while at it, please use "*ret" as the argument > to sizeof. The reason is slightly better maintainability, as then the type > of ret can be changed at the declaration and the sizeof computation is still > correct. Will do. >> + ret->queue = diff_queued_diff.queue; >> + ret->nr = diff_queued_diff.nr; >> + /* Ignore diff_queued_diff.alloc; we won't be changing size at all */ >> + >> + opts.output_format = DIFF_FORMAT_NO_OUTPUT; >> + diff_queued_diff.nr = 0; >> + diff_queued_diff.queue = NULL; >> + diff_flush(&opts); > > The comment is rather meant for the later lines or the former lines > (where ret is assigned), the empty line seems like it could go before > the comment? Perhaps I should just replaced the first three lines, including the comment, with *ret = diff_queued_diff; ? I was probably thinking along that track, which is why I had the comment grouped with lines above it, but was for whatever reason just assigning each value and then noting that one of them was actually unnecessary. But it wouldn't hurt to copy either.
