Hello Karina,
30.06.2023 17:25, Karina Litskevich wrote:
Hi,
Alexander wrote:
It also aligns the code with print_unaligned_vertical(), but I can't see why
need_recordsep = true;
is a no-op here (scan-build dislikes only need_recordsep = false;).
I suspect that removing that line will change the behaviour in cases when
need_recordsep = false after the loop "print cells" and the loop
"for (footers)" is executed.
As I understand cont->cells is supoused to have all cont->ncolumns * cont->nrows
entries filled so the loop "print cells" always assigns need_recordsep = true,
except when there are no cells at all or cancel_pressed == true.
If cancel_pressed == true then footers are not printed. So to have
need_recordsep == false before the loop "for (footers)" table should be empty,
and need_recordsep should be false before the loop "print cells". It can only
be false there when cont->opt->start_table == true and opt_tuples_only == true
so that headers are not printed. But when opt_tuples_only == true footers are
not printed either.
So technically removing "need_recordsep = true;" won't change the outcome. But
it's not obvious, so I also have doubts about removing this line. If someday
print options are changed, for example to support printing footers and not
printing headers, or anything else changes in this function, the output might
be unexpected with this line removed.
I think that the question that should be answered before moving forward
here is: what this discussion is expected to result in?
If the goal is to avoid an unused value to make Coverity/clang`s scan-build
a little happier, then maybe just don't touch other line, that is not
recognized as dead (at least by scan-build; I wonder what Coverity says
about that line).
Otherwise, if the goal is to do review the code and make it cleaner, then
why not get rid of "if (need_recordsep)" there?
Best regards,
Alexander
