Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?) - Mailing list pgsql-hackers

Andres Freund <andres@anarazel.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