Thread: Wrong dead return value in jsonb_utils.c

Wrong dead return value in jsonb_utils.c

From
Rikard Falkeborn
Date:
Returning -1 from a function with bool as return value is the same as returning true. Now, the code is dead (since elog(ERROR, ...) does not return) so it doesn't matter to the compiler, but changing to false is less confusing for the programmer. Appologies if this is seen as unnecessary churn.

The same code is present since 9.4, but perhaps it's not really worth backporting since it is more of an aesthetic change?
Attachment

Re: Wrong dead return value in jsonb_utils.c

From
Michael Paquier
Date:
On Sun, May 12, 2019 at 03:18:08AM +0200, Rikard Falkeborn wrote:
> Returning -1 from a function with bool as return value is the same as
> returning true. Now, the code is dead (since elog(ERROR, ...) does not
> return) so it doesn't matter to the compiler, but changing to false is less
> confusing for the programmer. Appologies if this is seen as unnecessary
> churn.
>
> The same code is present since 9.4, but perhaps it's not really worth
> backporting since it is more of an aesthetic change?

This is an aesthetic change in the fact that elog(ERROR) would not
cause -1 to be returned, still I agree that it is cleaner to do things
the way your patch does.  And the origin of the issue is I think that
the code of equalsJsonbScalarValue() has been copy-pasted from
compareJsonbScalarValue().

As that's really cosmetic, I would just change that on HEAD, or
perhaps others feel differently?
--
Michael

Attachment

Re: Wrong dead return value in jsonb_utils.c

From
Tom Lane
Date:
Michael Paquier <michael@paquier.xyz> writes:
> As that's really cosmetic, I would just change that on HEAD, or
> perhaps others feel differently?

+1 for a HEAD-only change.  I think the only really good arguments
for back-patching would be if this were causing compiler warnings
(but we've seen none) or if we thought it would likely lead to
hazards for back-patching future bug fixes (but the adjacent lines
seem unlikely to change).

            regards, tom lane



Re: Wrong dead return value in jsonb_utils.c

From
Michael Paquier
Date:
On Sun, May 12, 2019 at 10:15:05AM -0400, Tom Lane wrote:
> +1 for a HEAD-only change.  I think the only really good arguments
> for back-patching would be if this were causing compiler warnings
> (but we've seen none) or if we thought it would likely lead to
> hazards for back-patching future bug fixes (but the adjacent lines
> seem unlikely to change).

If it were to generate warnings, we would have already caught them as
this comes from 1171dbd.  Committed to HEAD.
--
Michael

Attachment