public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Date: Mon, 27 Feb 2023 10:30:42 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAAKRu_Yci1Pw5qnoYdPz-FFqX-qpS0y0R_TA=rA_njACGp-SGQ@mail.gmail.com>
References: <CAAKRu_af0YcV9CXG9M5R06tYRUzk13+H7qOsU6etmS4zE7RAUQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAAKRu_Yci1Pw5qnoYdPz-FFqX-qpS0y0R_TA=rA_njACGp-SGQ@mail.gmail.com>

Melanie Plageman <[email protected]> writes:
> Attached is a patch to remove the *_FIRST macros.
> I was going to add in code to change

>     for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
>     to
>     for (IOObject io_object = 0; (int) io_object < IOOBJECT_NUM_TYPES; io_object++)

I don't really like that proposal.  ISTM it's just silencing the
messenger rather than addressing the underlying problem, namely that
there's no guarantee that an IOObject variable can hold the value
IOOBJECT_NUM_TYPES, which it had better do if you want the loop to
terminate.  Admittedly it's quite unlikely that these three enums would
grow to the point that that becomes an actual hazard for them --- but
IMO it's still bad practice and a bad precedent for future code.

> but then I couldn't remember why we didn't just do

>     for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)

> I recall that when passing that loop variable into a function I was
> getting a compiler warning that required me to cast the value back to an
> enum to silence it:

>             pgstat_tracks_io_op(bktype, (IOObject) io_object,
> io_context, io_op))

> However, I am now unable to reproduce that warning.
> Moreover, I see in cases like table_block_relation_size() with
> ForkNumber, the variable i is passed with no cast to smgrnblocks().

Yeah, my druthers would be to just do it the way we do comparable
things with ForkNumber.  I don't feel like we need to invent a
better way here.

The risk of needing to cast when using the "int" loop variable
as an enum is obviously the downside of that approach, but we have
not seen any indication that any compilers actually do warn.
It's interesting that you did see such a warning ... I wonder which
compiler you were using at the time?

			regards, tom lane

view thread (36+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox