From: | "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com> |
---|---|
To: | "Magnus Hagander" <magnus(at)hagander(dot)net> |
Cc: | "pgsql-patches" <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: WIP: guc enums |
Date: | 2008-03-04 21:35:27 |
Message-ID: | 47CDC09F.8000503@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2008-03-05 03:44:37 | Re: actualized SQL/PSM patch |
Previous Message | Zoltan Boszormenyi | 2008-03-04 20:52:25 | Re: 64-bit CommandIds |