Chao Li <[email protected]> 于2026年2月27日周五 09:34写道：
> > > > On Feb 26, 2026, at 20:37, Álvaro Herrera <[email protected]> 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 > > >
v2-0001-guc-Clarify-dereference-order-in-newval-string-ch.patch
Description: Binary data
v2-0002-datetime-Clarify-DecodeInterval-field-parameter-t.patch
Description: Binary data
