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.