Re: WIP: guc enums - Mailing list pgsql-patches
From | Heikki Linnakangas |
---|---|
Subject | Re: WIP: guc enums |
Date | |
Msg-id | 47CDC09F.8000503@enterprisedb.com Whole thread Raw |
In response to | WIP: guc enums (Magnus Hagander <magnus@hagander.net>) |
Responses |
Re: WIP: guc enums
(Tom Lane <tgl@sss.pgh.pa.us>)
Re: WIP: guc enums (Magnus Hagander <magnus@hagander.net>) |
List | pgsql-patches |
Magnus Hagander wrote: > The patch only converts a couple of the potential enum variables to the new > type, mainly as a proof of concept. But already I hit the problem twice - > the variable that holds the value of the guc enum is a C enum itself, which > gives a compiler warning when I pass a pointer to it for > config_enum.variable. (in this case, Log_error_verbosity and log_statement > are enums and have the problem, but client_min_messages, log_min_messages > and log_min_error_statement are already int and don't have it) > > On my platform (linux x86) it works fine when I just cast this to (int *), > but I'm unsure if that's going to be safe on other platforms. I had some > indication that it's probably not? No, I don't think that's safe. Some googleing (*) suggests that the compiler is free to choose any integer type for an enum. If you do "*((int *)enumptr) = x", and the compiler chose a 16-bit type for the enum, you overwrite some unrelated piece of memory. > And if not, the only way I know to do it is to change the C level enums to > be an int and use #define:s instead of what's there now. If that's > required, is that an acceptable change in order to implement this? If not, > any better ideas on how to do it? Yuck :-(. We could keep using the assignment hooks. But they could be a lot simpler than they are nowadays, if the string -> int conversion was done by the GUC machinery: static const char * assign_client_min_messages(int newval, bool doit, GucSource source) { client_min_messages = newval; } Another idea would be cajole the compiler to choose a type of the same length as "int", by adding a dummy enum value to the enum, like: enum client_min_messages { DEBUG, INFO, ..., DUMMY = INT_MAX } Though I guess it might in theory choose something even wider, and the "*((int *)enumptr) = x" would fail to set all the bits of the enum variable. BTW, shouldn't be using malloc in config_enum_get_options... (*): http://david.tribble.com/text/cdiffs.htm#C99-enum-type and what I believe to be the current C99 standard, see "6.7.2.2 Enumeration specifiers": http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
pgsql-patches by date: