-Wcast-qual cleanup, part 1

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

Responses

Browse pgsql-hackers by date

  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