Em sex., 30 de jun. de 2023 às 13:00, Alexander Lakhin <exclus...@gmail.com>
escreveu:
> 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.
>
> Hi Alexander,
> I think that the question that should be answered before moving forward
> here is: what this discussion is expected to result in?
>
I hope to make the function more readable and maintainable.
> 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).
>
if (cont->opt->stop_table)
477 {
478 printTableFooter *footers = footers_with_default(cont);
479
480 if (!opt_tuples_only && footers != NULL && !
cancel_pressed)
481 {
482 printTableFooter *f;
483
484 for (f = footers; f; f = f->next)
485 {
486 if (need_recordsep)
487 {
488 print_separator(cont->opt->
recordSep, fout);
CID 1512766 (#1 of 1): Unused value (UNUSED_VALUE)assigned_value: Assigning
value false to need_recordsep here, but that stored value is overwritten
before it can be used.
489 need_recordsep = false;
490 }
491 fputs(f->data, fout);
value_overwrite: Overwriting previous write to need_recordsep with value
true.
492 need_recordsep = true;
493 }
494 }
495
> Otherwise, if the goal is to do review the code and make it cleaner, then
> why not get rid of "if (need_recordsep)" there?
>
I don't agree with removing this line, as it is essential to print the
separators, in the footers.
best regards,
Ranier Vilela
