Bruce Momjian wrote:
>I understand the desire to deal with this later, but we never seem to
>focus on compiler issues except during beta.
>
>I think I know why we are hitting the warning in tablecmds.c and not in
>trigger.c. Trigger.c has:
>
> ExecCallTriggerFunc(TriggerData *trigdata,
> FmgrInfo *finfo,
> MemoryContext per_tuple_context)
> {
>
> ...
> fcinfo.flinfo = finfo;
> fcinfo.context = (Node *) trigdata;
>
>This does not generate a warning, while this does in tablecmds.c:
>
> while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
> {
> FunctionCallInfoData fcinfo;
> TriggerData trigdata;
>
> /*
> * Make a call to the trigger function
> *
> * No parameters are passed, but we do set a context
> */
> MemSet(&fcinfo, 0, sizeof(fcinfo));
>
> /*
> * 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);
> }
>
>trigger.c doesn't generate the warning because it is a TriggerData*,
>while tablecmds.c has an acual structure. trigger.c doesn't know if the
>passed pointer was allocated via malloc(), and therefore properly
>aligned for any data type, or if it was allocated on the stack, so it
>does not complain.
>
>
I could be wrong, but I don't think it has to do with malloc or whether
or not it is on the stack or in dynamic memory, but rather to do with
the fact that you have manipulated it using one personality and then
cast it to another.
>In tablecmds.c, they create it as local loop structure that is passed to
>RI_FKey_check_ins(). What we should be doing in this code is allocating
>a proper Node structure using makeNode(TriggerData), and using that in
>the loop. (We should allocate it outside the loop.)
>
>I see the same problem in execQual.c. sysv_shmem probably needs the
>void*, and psql/command.c should have parse_char defines as unsigned
>char **, rather than char **.
>
>
signedness is not supposed to affect strict aliasing, but the compiler
could be confused.
(disclaimer) I am not a language lawyer, and this is one of those rare
times we need one :-) Don't take my word for it on this stuff.
cheers
andrew
