From: | Peter Eisentraut <peter_e(at)gmx(dot)net> |
---|---|
To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | -Wcast-qual cleanup, part 1 |
Date: | 2011-11-07 05:13:05 |
Message-ID: | 1320642785.25734.13.camel@vanquo.pezone.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I've been looking into getting rid of the warnings pointed out by
-Wcast-qual, which are mainly caused by casting away "const" qualifiers.
(It also warns about casting away "volatile" qualifiers, and we have a
number of those as well, but that's for another day.) I've gotten them
down to a manageable amount, and with a bit of effort we could probably
get them down to a few dozen.
I have split my change set into about a dozen separate patches, which
I'll post for discussion in separate batches. But I'll start with the
biggest one, which includes the following notable interwoven changes:
The error callbackup functions signature changes thus:
static void pg_start_backup_callback(int code, Datum arg);
static bool read_backup_label(XLogRecPtr *checkPointLoc,
bool *backupEndRequired);
-static void rm_redo_error_callback(void *arg);
+static void rm_redo_error_callback(const void *arg);
static int get_sync_bit(int method);
This is because various places provide const values as the callback
argument.
The xlog _desc() functions signature changes thus:
}
void
-gin_desc(StringInfo buf, uint8 xl_info, char *rec)
+gin_desc(StringInfo buf, uint8 xl_info, const char *rec)
{
uint8 info = xl_info & ~XLR_INFO_MASK;
This is because rm_redo_error_callback() makes calls to the _desc()
functions.
A couple of themes that also appear in the other patches are worth
thinking about:
1. Hiding away the pointerness of a type behind a typedef like this
typedef struct CopyStateData *CopyState;
is no good, because that loses the ability to apply const (or other)
qualifiers to the pointer. See this hunk:
* The argument for the error context must be CopyState.
*/
void
-CopyFromErrorCallback(void *arg)
+CopyFromErrorCallback(const void *arg)
{
- CopyState cstate = (CopyState) arg;
+ const CopyStateData *cstate = (const CopyStateData *) arg;
if (cstate->binary)
{
I think we recently had another discussion (Relation/RelationData?) that
also concluded that this coding style is flawed.
2. Macros accessing structures should come in two variants: a "get"
version, and a "set"/anything else version, so that the "get" version
can preserve the const qualifier. See this hunk:
#define SizeOfXLogRecord MAXALIGN(sizeof(XLogRecord))
#define XLogRecGetData(record) ((char*) (record) + SizeOfXLogRecord)
+#define XLogRecGetConstData(record) ((const char*) (record) +
SizeOfXLogRecord)
/*
* XLOG uses only low 4 bits of xl_info. High 4 bits may be used by rmgr.
(This is not a particularly good naming convention. I would probably go
with XLogRecData() and XLogRecGetData() if we were designing this from
scratch.)
Anyway, attached is the first patch for your amusement.
Attachment | Content-Type | Size |
---|---|---|
cast-qual-xlog-errcallbacks.patch | text/x-patch | 55.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | J Smith | 2011-11-07 05:52:42 | Re: unaccent extension missing some accents |
Previous Message | Jan Kundrát | 2011-11-07 05:02:27 | [patch] Include detailed information about a row failing a CHECK constraint into the error message |