From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, melanieplageman(at)gmail(dot)com, vignesh21(at)gmail(dot)com, pryzby(at)telsasoft(dot)com, lukas(at)fittl(dot)com, alvherre(at)alvh(dot)no-ip(dot)org, magnus(at)hagander(dot)net, pgsql-hackers(at)postgresql(dot)org, thomas(dot)munro(at)gmail(dot)com, m(dot)sakrejda(at)gmail(dot)com |
Subject: | Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?) |
Date: | 2023-02-26 18:20:00 |
Message-ID: | 20520.1677435600@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andres Freund <andres(at)anarazel(dot)de> writes:
> Pushed the first (and biggest) commit. More tomorrow.
I hadn't run my buildfarm-compile-warning scraper for a little while,
but I just did, and I find that this commit is causing warnings on
no fewer than 14 buildfarm animals. They all look like
ayu | 2023-02-25 23:02:08 | pgstat_io.c:40:14: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:43:16: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:70:19: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:71:20: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:115:14: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstat_io.c:118:16: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstatfuncs.c:1329:12: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
ayu | 2023-02-25 23:02:08 | pgstatfuncs.c:1334:17: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
That is, these compilers think that comparisons like
io_object < IOOBJECT_NUM_TYPES
io_context < IOCONTEXT_NUM_TYPES
are constant-true. This seems not good; if they were to actually
act on this observation, by removing those loop-ending tests,
we'd have a problem.
The issue seems to be that code like this:
typedef enum IOContext
{
IOCONTEXT_BULKREAD,
IOCONTEXT_BULKWRITE,
IOCONTEXT_NORMAL,
IOCONTEXT_VACUUM,
} IOContext;
#define IOCONTEXT_FIRST IOCONTEXT_BULKREAD
#define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)
is far too cute for its own good. I'm not sure about how to fix it
either. I thought of defining
#define IOCONTEXT_LAST IOCONTEXT_VACUUM
and make the loop conditions like "io_context <= IOCONTEXT_LAST",
but that doesn't actually fix the problem.
(Even aside from that, I do not find this coding even a little bit
mistake-proof: you still have to remember to update the #define
when adding another enum value.)
We have similar code involving enum ForkNumber but it looks to me
like the loop variables are always declared as plain "int". That
might be the path of least resistance here.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-02-26 18:52:47 | Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?) |
Previous Message | Andres Freund | 2023-02-26 18:00:29 | Re: stopgap fix for signal handling during restore_command |