Re: guc: make dereference style consistent in check_backtrace_functions - Mailing list pgsql-hackers

> On Mar 2, 2026, at 11:17, zhanghu <kongbaik228@gmail.com> wrote:
>
> zhanghu <kongbaik228@gmail.com> 于2026年2月27日周五 16:46写道：
>>
>>
>>
>> Chao Li <li.evan.chao@gmail.com> 于2026年2月27日周五 09:34写道：
>>>
>>>
>>>
>>>> On Feb 26, 2026, at 20:37, Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>>>
>>>> There is at least one more place in the code where this is done.
>>>>
>>>
>>> I did a search with the command: grep -RInE '\*[[:space:]]*[A-Za-z_][A-Za-z0-9_]*\[0\]' src contrib --include='*.c'
>>>
>>> Excluding irrelevant results, there are 3 more occurrences:
>>>
>>> 1 - contrib/basic_archive/basic_archive.c line 105
>>> ```
>>>        if (*newval == NULL || *newval[0] == '\0')
>>>                return true;
>>> ```
>>>
>>> Here, the code checks *newval first, which implies that the subsequent *newval[0] is unintentional syntax.
>>>
>>> 2 - src/interfaces/ecpg/pgtypeslib/interval.c line 62
>>> ```
>>> int
>>> DecodeInterval(char **field, int *ftype, int nf,        /* int range, */
>>>                           int *dtype, struct /* pg_ */ tm *tm, fsec_t *fsec)
>>> {
>>>  ...
>>>        if (IntervalStyle == INTSTYLE_SQL_STANDARD && *field[0] == '-')
>>>        {
>>>                /* Check for additional explicit signs */
>>>                bool            more_signs = false;
>>>
>>>                for (i = 1; i < nf; i++)
>>>                {
>>>                        if (*field[i] == '-' || *field[i] == '+')
>>>                        {
>>>                                more_signs = true;
>>>                                break;
>>>                        }
>>>                }
>>> ```
>>>
>>> 3 - src/backend/utils/adt/datatime.c line 3522
>>> ```
>>> int
>>> DecodeInterval(char **field, int *ftype, int nf, int range,
>>>                           int *dtype, struct pg_itm_in *itm_in)
>>> {
>>> ...
>>>        if (IntervalStyle == INTSTYLE_SQL_STANDARD && nf > 0 && *field[0] == '-')
>>>        {
>>>                force_negative = true;
>>>                /* Check for additional explicit signs */
>>>                for (i = 1; i < nf; i++)
>>>                {
>>>                        if (*field[i] == '-' || *field[i] == '+')
>>>                        {
>>>                                force_negative = false;
>>>                                break;
>>>                        }
>>>                }
>>>        }
>>> ```
>>>
>>> Where 2&3 makes this patch more interesting.
>>>
>>> Both occurrences are inside functions named DecodeInterval. For non-zero i, the code also performs *field[i]:
>>>
>>> Given this code has been there for years, I don’t believe it is a bug. I checked the callers of DecodeInterval in
bothfiles and found that field is defined as: 
>>> ```
>>>    char *field[MAXDATEFIELDS];
>>> ```
>>>
>>> This explains why *field[i] works; it is doing the intended thing by getting the first character of the string at
arrayposition i. 
>>>
>>> However, since the precedence between the [] and * operators frequently confuses people, I suggest adding
parenthesesto make the intention explicit as *(field[i]). Furthermore, I think we should change the function signatures
touse the type char *field[] to reflect the actual type the functions expect. If a caller were to pass a true char **
typedfield to DecodeInterval, the current logic would result in a bug. 
>>>
>>> See the attached diff for my suggested changes.
>>>
>>> Best regards,
>>> --
>>> Chao Li (Evan)
>>> HighGo Software Co., Ltd.
>>> https://www.highgo.com/
>>>
>>> Hi,
>>>
>>> Thank you all for the reviews and detailed feedback.
>>>
>>> Álvaro, thanks for pointing out that there were additional
>>> occurrences elsewhere in the tree. I have updated the original
>>> patch to address those cases; the revised version is attached
>>> as v2-0001.
>>>
>>> I also appreciate the  review and suggestions from
>>> Chao and Junwang.
>>>
>>> Regarding the additional changes suggested by Chao: they go
>>> somewhat beyond the original scope of my original patch.
>>> To keep the discussion concrete, I have included Chao’s proposed
>>> diff as a separate patch (v2-0002) so it can be reviewed independently.
>>>
>>> I have reviewed v2-0002 locally, and it looks good to me.
>>>
>>> Thanks again for the guidance.
>>>
>>> Regards,
>>> Zhang Hu
>>>
>>>
>
> Hi,
>
> I am planning to add this patch to the current CommitFest, but when
> logging in to commitfest.postgresql.org I get the message:
>
> “You have not passed the cool off period yet.”
>
> It seems my account is still within the cool-off period after registration.
>
> Could someone please add this patch to the CommitFest on my behalf?
>
> Thanks.
>
> Best regards,
> Zhang Hu

Yes, there is a cool off period when one first time registers to the CommitFest. I don’t remember exactly how many days
theperiod is, should be just a few days. So stay tuned. 

I tried to add the patch to CF, but I noticed that, if I do that, the patch author would be me, and as you are fully
registered,I could not change the author to you. So, please just wait to pass the cool off period and then create the
CFentry. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/