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

From Chao Li
Subject Re: guc: make dereference style consistent in check_backtrace_functions
Date
Msg-id 63229AD5-48DB-417C-9361-EA478DAF57AF@gmail.com
Whole thread Raw
In response to Re: guc: make dereference style consistent in check_backtrace_functions  (Álvaro Herrera <alvherre@kurilemu.de>)
Responses Re: guc: make dereference style consistent in check_backtrace_functions
List pgsql-hackers

> 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 both
filesand 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
positioni. 

However, since the precedence between the [] and * operators frequently confuses people, I suggest adding parentheses
tomake the intention explicit as *(field[i]). Furthermore, I think we should change the function signatures to use the
typechar *field[] to reflect the actual type the functions expect. If a caller were to pass a true char ** typed field
toDecodeInterval, 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/





Attachment

pgsql-hackers by date:

Previous
From: Andreas Karlsson
Date:
Subject: Re: Use pg_malloc macros in src/fe_utils
Next
From: Richard Guo
Date:
Subject: Re: Convert ALL SubLinks to ANY SubLinks