On Wed, Sep 21, 2016 at 3:40 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Wed, Sep 21, 2016 at 8:13 AM, Thomas Munro
> <thomas.mu...@enterprisedb.com> wrote:
>> It looks like this array wants to be in alphabetical order, but it
>> isn't quite. Also, perhaps a compile time assertion about the size of
>> the array matching EVENT_LAST_TYPE could be useful?
>
> In GetWaitEventIdentifier()? I'd think that just returning ??? would
> have been fine if there is a non-matching call.
Yeah but that's at run time. I meant you could help developers
discover ASAP if they add a new item to one place but not the other
with a compile time assertion:
const char *
GetWaitEventIdentifier(uint16 eventId)
{
StaticAssertStmt(lengthof(WaitEventNames) == WE_LAST_TYPE + 1,
"WaitEventNames must match WaitEventIdentifiers");
if (eventId > WE_LAST_TYPE)
return "???";
return WaitEventNames[eventId];
}
>> +1 from me too for avoiding the overly general term 'event'. It does
>> seem a little odd to leave the enumerators names as EVENT_... though;
>> shouldn't these be WAIT_EVENT_... or WE_...? Or perhaps you could
>> consider WaitPointIdentifier and WP_SECURE_READ or
>> WaitEventPointIdentifier and WEP_SECURE_READ, if you buy my earlier
>> argument that what we are really naming here is point in the code
>> where we wait, not the events we're waiting for. Contrast with
>> LWLocks where we report the lock that you're waiting for, not the
>> place in the code where you're waiting for that lock.
>
> Well, WE_ if I need make a choice for something else than EVENT_.
You missed a couple that are hiding inside #ifdef WIN32:
>From pgstat.c:
+ EVENT_PGSTAT_MAIN);
>From syslogger.c:
+ EVENT_SYSLOGGER_MAIN);
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
