Re: guc: make dereference style consistent in check_backtrace_functions

> On Mar 2, 2026, at 11:17, zhanghu <kongbaik228(at)gmail(dot)com> wrote:
>
> zhanghu <kongbaik228(at)gmail(dot)com> 于2026年2月27日周五 16:46写道：
>>
>>
>>
>> Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> 于2026年2月27日周五 09:34写道：
>>>
>>>
>>>
>>>> On Feb 26, 2026, at 20:37, Álvaro Herrera <alvherre(at)kurilemu(dot)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 both files 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 array position i.
>>>
>>> However, since the precedence between the [] and * operators frequently confuses people, I suggest adding parentheses to make the intention explicit as *(field[i]). Furthermore, I think we should change the function signatures to use the type char *field[] to reflect the actual type the functions expect. If a caller were to pass a true char ** typed field 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 the period 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 CF entry.

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