new warnings with clang-21 / how const is Datum - Mailing list pgsql-hackers

From Peter Eisentraut
Subject new warnings with clang-21 / how const is Datum
Date
Msg-id 6604ad6e-5934-43ac-8590-15113d6ae4b1@eisentraut.org
Whole thread Raw
Responses Re: new warnings with clang-21 / how const is Datum
List pgsql-hackers
clang-21 shows some new warnings:

../src/backend/access/common/toast_internals.c:296:33: error: variable 
'chunk_data' is uninitialized when passed as a const pointer argument 
here [-Werror,-Wuninitialized-const-pointer]
   296 |         t_values[2] = PointerGetDatum(&chunk_data);

../src/backend/access/gist/gistutil.c:207:28: error: variable 'attrsize' 
is uninitialized when passed as a const pointer argument here 
[-Werror,-Wuninitialized-const-pointer]
   207 | 
                 PointerGetDatum(&attrsize));
       | 
                                  ^~~~~~~~
../src/backend/access/gist/gistutil.c:276:27: error: variable 'dstsize' 
is uninitialized when passed as a const pointer argument here 
[-Werror,-Wuninitialized-const-pointer]
   276 | 
  PointerGetDatum(&dstsize));
       | 
                   ^~~~~~~


The first one is easily fixed by re-arranging the code a bit.

The other two indicate more fundamental problems.  The gist API uses 
these arguments to pass back information; these pointers do not in fact 
point to a const object.

This possibly means that the change

commit c8b2ef05f48
Author: Peter Eisentraut <peter@eisentraut.org>
Date:   Tue Sep 27 20:47:07 2022

     Convert *GetDatum() and DatumGet*() macros to inline functions

that made PointerGetDatum() look like

     static inline Datum
     PointerGetDatum(const void *X)

was mistaken in adding the const qualifier.

We could remove that qualifier (this would propagate to several other 
functions built on top of PointerGetDatum()), but then there will be 
complaints from the other side:

     static const TableAmRoutine heapam_methods = {

     PG_RETURN_POINTER(&heapam_methods);

Then the question is, which one of these should be considered the outlier?

We could remove the const qualifications from the API and stick an 
unconstify() around &heapam_methods.

Or we could maybe make a new function PointerNonConstGetDatum() that we 
use for exceptional cases like the gist API.

There is a related issue that I have been researching for some time.  It 
seems intuitively correct that the string passed into a data type input 
function should not be modified by that function.  And so the relevant 
functions could use const qualifiers like

extern Datum InputFunctionCall(FmgrInfo *flinfo, const char *str, ...);

which they currently do not.

There are a some places in the code where const strings get passed into 
some *InputFunctionCall() functions and have their const qualifications 
rudely cast away, which seems unsatisfactory.

Overall, the question to what extent fmgr functions are allowed to 
modify objects pointed to by their input arguments doesn't seem to be 
addressed anywhere.




pgsql-hackers by date:

Previous
From: jian he
Date:
Subject: Re: support fast default for domain with constraints
Next
From: John Naylor
Date:
Subject: Re: Generate GUC tables from .dat file