Thread: Wrong dead return value in jsonb_utils.c

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

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

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