Re: Avoid unused value (src/fe_utils/print.c) - Mailing list pgsql-hackers

From Alexander Lakhin
Subject Re: Avoid unused value (src/fe_utils/print.c)
Date
Msg-id 2cec5e2e-2339-6035-f0f4-60b5778cfe3b@gmail.com
Whole thread Raw
In response to Re: Avoid unused value (src/fe_utils/print.c)  (Karina Litskevich <litskevichkarina@gmail.com>)
Responses Re: Avoid unused value (src/fe_utils/print.c)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: ProcessStartupPacket(): database_name and user_name truncation
Next
From: Ranier Vilela
Date:
Subject: Re: Avoid unused value (src/fe_utils/print.c)