Re: fix for strict-alias warnings - Mailing list pgsql-patches

The warning is this:

command.c: In function `unescape':
command.c:1283: warning: dereferencing type-punned pointer will break
strict-aliasing rules

p is declared thus:

        const unsigned char *p;

If I change common.c/h so that parse_char() takes an (unsigned char **)
argument, cast its 2nd argument to the call to strtol to  (char **), and
change the cast in the call to (unsigned char **) those warnings go away,
but I get one from the other place parse_char() is called, namely prompt.c,
which has similar code but this time p is not declared as unsigned. - and so
it goes.

Grrr. What a pain.

andrew


----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: "Andrew Dunstan" <andrew@dunslane.net>
Cc: "PG Patches" <pgsql-patches@postgresql.org>
Sent: Sunday, October 12, 2003 7:18 PM
Subject: Re: [PATCHES] fix for strict-alias warnings


> Andrew Dunstan wrote:
> >
> > ----- Original Message -----
> > From: "Bruce Momjian" <pgman@candle.pha.pa.us>
> > >
> > > I have backed out the patch.
> > >
> > > Looking at the case in tablecmds.c and proc.c, the first was assigning
a
> > > struct with a NodeTag pointer as its first element to another struct
> > > with NodeTag as its first element.  In fact, we do this all over the
> > > place, having different structure pointers with a start element of
> > > NodeTag.
>
> I have attached and applied the following patch to use makeNode for
> structures that will later be cast to Node*, rather than having them be
> allocated as stack variables.
>
> This leaves the only remaning compiler warning coming from common.c listed
> below.  What is the exact warning generated --- this seems like a
> different issue.
>
> --------------------------------------------------------------------------
-
>
> Index: src/bin/psql/command.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/command.c,v
> retrieving revision 1.103
> diff -c -w -r1.103 command.c
> *** src/bin/psql/command.c 29 Sep 2003 16:39:18 -0000 1.103
> --- src/bin/psql/command.c 11 Oct 2003 13:50:15 -0000
> ***************
> *** 1280,1286 ****
>   case '7':
>   case '8':
>   case '9':
> ! c = parse_char((char **) &p);
>   break;
>
>   default:
> --- 1280,1286 ----
>   case '7':
>   case '8':
>   case '9':
> ! c = parse_char((void *) &p);
>   break;
>
>   default:
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania
19073
>


----------------------------------------------------------------------------
----


> Index: src/backend/commands/tablecmds.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/commands/tablecmds.c,v
> retrieving revision 1.88
> diff -c -c -r1.88 tablecmds.c
> *** src/backend/commands/tablecmds.c 11 Oct 2003 18:04:25 -0000 1.88
> --- src/backend/commands/tablecmds.c 12 Oct 2003 23:10:21 -0000
> ***************
> *** 3449,3454 ****
> --- 3449,3455 ----
>   Relation pkrel)
>   {
>   HeapScanDesc scan;
> + TriggerData *trigdata = makeNode(TriggerData); /* must be Node aligned
*/
>   HeapTuple tuple;
>   Trigger trig;
>   List    *list;
> ***************
> *** 3506,3512 ****
>   while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
>   {
>   FunctionCallInfoData fcinfo;
> - TriggerData trigdata;
>
>   /*
>   * Make a call to the trigger function
> --- 3507,3512 ----
> ***************
> *** 3518,3537 ****
>   /*
>   * We assume RI_FKey_check_ins won't look at flinfo...
>   */
> ! trigdata.type = T_TriggerData;
> ! trigdata.tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_ROW;
> ! trigdata.tg_relation = rel;
> ! trigdata.tg_trigtuple = tuple;
> ! trigdata.tg_newtuple = NULL;
> ! trigdata.tg_trigger = &trig;
>
> ! fcinfo.context = (Node *) &trigdata;
>
>   RI_FKey_check_ins(&fcinfo);
>   }
>
>   heap_endscan(scan);
>
>   pfree(trig.tgargs);
>   }
>
> --- 3518,3538 ----
>   /*
>   * We assume RI_FKey_check_ins won't look at flinfo...
>   */
> ! trigdata->type = T_TriggerData;
> ! trigdata->tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_ROW;
> ! trigdata->tg_relation = rel;
> ! trigdata->tg_trigtuple = tuple;
> ! trigdata->tg_newtuple = NULL;
> ! trigdata->tg_trigger = &trig;
>
> ! fcinfo.context = (Node *) trigdata;
>
>   RI_FKey_check_ins(&fcinfo);
>   }
>
>   heap_endscan(scan);
>
> + pfree(trigdata);
>   pfree(trig.tgargs);
>   }
>
> Index: src/backend/executor/execQual.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/executor/execQual.c,v
> retrieving revision 1.148
> diff -c -c -r1.148 execQual.c
> *** src/backend/executor/execQual.c 11 Oct 2003 18:04:25 -0000 1.148
> --- src/backend/executor/execQual.c 12 Oct 2003 23:10:23 -0000
> ***************
> *** 699,705 ****
>   List    *arguments = fcache->args;
>   Datum result;
>   FunctionCallInfoData fcinfo;
> ! ReturnSetInfo rsinfo; /* for functions returning sets */
>   ExprDoneCond argDone;
>   bool hasSetArg;
>   int i;
> --- 699,706 ----
>   List    *arguments = fcache->args;
>   Datum result;
>   FunctionCallInfoData fcinfo;
> ! /* for functions returning sets, must be aligned as Node, so use
makeNode */
> ! ReturnSetInfo *rsinfo = makeNode(ReturnSetInfo);
>   ExprDoneCond argDone;
>   bool hasSetArg;
>   int i;
> ***************
> *** 746,760 ****
>   */
>   if (fcache->func.fn_retset)
>   {
> ! fcinfo.resultinfo = (Node *) &rsinfo;
> ! rsinfo.type = T_ReturnSetInfo;
> ! rsinfo.econtext = econtext;
> ! rsinfo.expectedDesc = NULL;
> ! rsinfo.allowedModes = (int) SFRM_ValuePerCall;
> ! rsinfo.returnMode = SFRM_ValuePerCall;
>   /* isDone is filled below */
> ! rsinfo.setResult = NULL;
> ! rsinfo.setDesc = NULL;
>   }
>
>   /*
> --- 747,761 ----
>   */
>   if (fcache->func.fn_retset)
>   {
> ! fcinfo.resultinfo = (Node *) rsinfo;
> ! rsinfo->type = T_ReturnSetInfo;
> ! rsinfo->econtext = econtext;
> ! rsinfo->expectedDesc = NULL;
> ! rsinfo->allowedModes = (int) SFRM_ValuePerCall;
> ! rsinfo->returnMode = SFRM_ValuePerCall;
>   /* isDone is filled below */
> ! rsinfo->setResult = NULL;
> ! rsinfo->setDesc = NULL;
>   }
>
>   /*
> ***************
> *** 803,812 ****
>   if (callit)
>   {
>   fcinfo.isnull = false;
> ! rsinfo.isDone = ExprSingleResult;
>   result = FunctionCallInvoke(&fcinfo);
>   *isNull = fcinfo.isnull;
> ! *isDone = rsinfo.isDone;
>   }
>   else
>   {
> --- 804,813 ----
>   if (callit)
>   {
>   fcinfo.isnull = false;
> ! rsinfo->isDone = ExprSingleResult;
>   result = FunctionCallInvoke(&fcinfo);
>   *isNull = fcinfo.isnull;
> ! *isDone = rsinfo->isDone;
>   }
>   else
>   {
> ***************
> *** 903,909 ****
>   TupleDesc tupdesc = NULL;
>   Oid funcrettype;
>   FunctionCallInfoData fcinfo;
> ! ReturnSetInfo rsinfo;
>   MemoryContext callerContext;
>   MemoryContext oldcontext;
>   TupleTableSlot *slot;
> --- 904,910 ----
>   TupleDesc tupdesc = NULL;
>   Oid funcrettype;
>   FunctionCallInfoData fcinfo;
> ! ReturnSetInfo *rsinfo = makeNode(ReturnSetInfo); /* must be Node aligned
*/
>   MemoryContext callerContext;
>   MemoryContext oldcontext;
>   TupleTableSlot *slot;
> ***************
> *** 992,1006 ****
>   * doesn't actually get to see the resultinfo, but set it up anyway
>   * because we use some of the fields as our own state variables.
>   */
> ! fcinfo.resultinfo = (Node *) &rsinfo;
> ! rsinfo.type = T_ReturnSetInfo;
> ! rsinfo.econtext = econtext;
> ! rsinfo.expectedDesc = expectedDesc;
> ! rsinfo.allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize);
> ! rsinfo.returnMode = SFRM_ValuePerCall;
>   /* isDone is filled below */
> ! rsinfo.setResult = NULL;
> ! rsinfo.setDesc = NULL;
>
>   /*
>   * Switch to short-lived context for calling the function or
> --- 993,1007 ----
>   * doesn't actually get to see the resultinfo, but set it up anyway
>   * because we use some of the fields as our own state variables.
>   */
> ! fcinfo.resultinfo = (Node *) rsinfo;
> ! rsinfo->type = T_ReturnSetInfo;
> ! rsinfo->econtext = econtext;
> ! rsinfo->expectedDesc = expectedDesc;
> ! rsinfo->allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize);
> ! rsinfo->returnMode = SFRM_ValuePerCall;
>   /* isDone is filled below */
> ! rsinfo->setResult = NULL;
> ! rsinfo->setDesc = NULL;
>
>   /*
>   * Switch to short-lived context for calling the function or
> ***************
> *** 1028,1044 ****
>   if (direct_function_call)
>   {
>   fcinfo.isnull = false;
> ! rsinfo.isDone = ExprSingleResult;
>   result = FunctionCallInvoke(&fcinfo);
>   }
>   else
>   {
>   result = ExecEvalExpr(funcexpr, econtext,
> !   &fcinfo.isnull, &rsinfo.isDone);
>   }
>
>   /* Which protocol does function want to use? */
> ! if (rsinfo.returnMode == SFRM_ValuePerCall)
>   {
>   /*
>   * Check for end of result set.
> --- 1029,1045 ----
>   if (direct_function_call)
>   {
>   fcinfo.isnull = false;
> ! rsinfo->isDone = ExprSingleResult;
>   result = FunctionCallInvoke(&fcinfo);
>   }
>   else
>   {
>   result = ExecEvalExpr(funcexpr, econtext,
> !   &fcinfo.isnull, &rsinfo->isDone);
>   }
>
>   /* Which protocol does function want to use? */
> ! if (rsinfo->returnMode == SFRM_ValuePerCall)
>   {
>   /*
>   * Check for end of result set.
> ***************
> *** 1047,1053 ****
>   * tupdesc or tuplestore (since we can't get a tupdesc in the
>   * function-returning-tuple case)
>   */
> ! if (rsinfo.isDone == ExprEndResult)
>   break;
>
>   /*
> --- 1048,1054 ----
>   * tupdesc or tuplestore (since we can't get a tupdesc in the
>   * function-returning-tuple case)
>   */
> ! if (rsinfo->isDone == ExprEndResult)
>   break;
>
>   /*
> ***************
> *** 1093,1100 ****
>   }
>   tupstore = tuplestore_begin_heap(true, false, SortMem);
>   MemoryContextSwitchTo(oldcontext);
> ! rsinfo.setResult = tupstore;
> ! rsinfo.setDesc = tupdesc;
>   }
>
>   /*
> --- 1094,1101 ----
>   }
>   tupstore = tuplestore_begin_heap(true, false, SortMem);
>   MemoryContextSwitchTo(oldcontext);
> ! rsinfo->setResult = tupstore;
> ! rsinfo->setDesc = tupdesc;
>   }
>
>   /*
> ***************
> *** 1127,1139 ****
>   /*
>   * Are we done?
>   */
> ! if (rsinfo.isDone != ExprMultipleResult)
>   break;
>   }
> ! else if (rsinfo.returnMode == SFRM_Materialize)
>   {
>   /* check we're on the same page as the function author */
> ! if (!first_time || rsinfo.isDone != ExprSingleResult)
>   ereport(ERROR,
>   (errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
>   errmsg("table-function protocol for materialize mode was not
followed")));
> --- 1128,1140 ----
>   /*
>   * Are we done?
>   */
> ! if (rsinfo->isDone != ExprMultipleResult)
>   break;
>   }
> ! else if (rsinfo->returnMode == SFRM_Materialize)
>   {
>   /* check we're on the same page as the function author */
> ! if (!first_time || rsinfo->isDone != ExprSingleResult)
>   ereport(ERROR,
>   (errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
>   errmsg("table-function protocol for materialize mode was not
followed")));
> ***************
> *** 1144,1150 ****
>   ereport(ERROR,
>   (errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
>   errmsg("unrecognized table-function returnMode: %d",
> ! (int) rsinfo.returnMode)));
>
>   first_time = false;
>   }
> --- 1145,1151 ----
>   ereport(ERROR,
>   (errcode(ERRCODE_E_R_I_E_SRF_PROTOCOL_VIOLATED),
>   errmsg("unrecognized table-function returnMode: %d",
> ! (int) rsinfo->returnMode)));
>
>   first_time = false;
>   }
> ***************
> *** 1152,1159 ****
>   MemoryContextSwitchTo(callerContext);
>
>   /* The returned pointers are those in rsinfo */
> ! *returnDesc = rsinfo.setDesc;
> ! return rsinfo.setResult;
>   }
>
>
> --- 1153,1160 ----
>   MemoryContextSwitchTo(callerContext);
>
>   /* The returned pointers are those in rsinfo */
> ! *returnDesc = rsinfo->setDesc;
> ! return rsinfo->setResult;
>   }
>
>
> Index: src/backend/port/sysv_shmem.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/port/sysv_shmem.c,v
> retrieving revision 1.19
> diff -c -c -r1.19 sysv_shmem.c
> *** src/backend/port/sysv_shmem.c 11 Oct 2003 18:04:25 -0000 1.19
> --- src/backend/port/sysv_shmem.c 12 Oct 2003 23:10:24 -0000
> ***************
> *** 365,371 ****
>
>   if (hdr->magic != PGShmemMagic)
>   {
> ! shmdt(hdr);
>   return NULL; /* segment belongs to a non-Postgres app */
>   }
>
> --- 365,371 ----
>
>   if (hdr->magic != PGShmemMagic)
>   {
> ! shmdt((void *)hdr);
>   return NULL; /* segment belongs to a non-Postgres app */
>   }
>
>


----------------------------------------------------------------------------
----


>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match
>