Hi Tom,
Thanks for the review.
Generated a new patch v2 (attached) following your suggestion and
performed the same test again. The test results looks good including the
"make check".
On 2020-04-06 3:16 p.m., Tom Lane wrote:
> David Zhang <david.zhang@highgo.ca> writes:
>> I got an error when I was trying to insert a circle using the syntax
>> (the 3rd one) specified in the latest document.
> Hm. Presumably, that has never worked, and we've had no complaints
> to date. I'm halfway inclined to treat it as a documentation bug
> and remove the claim that it works.
>
>> The patch based on tag "REL_12_2" is attached.
> This patch looks extremely dangerous to me, because it'll allow "s"
> to get incremented past the ending nul character ... and then the
> code will proceed to keep scanning, which at best is useless and
> at worst will end in a core dump.
>
> What actually looks wrong to me in this code is the initial bit
>
> if ((*s == LDELIM_C) || (*s == LDELIM))
> {
> depth++;
> cp = (s + 1);
> while (isspace((unsigned char) *cp))
> cp++;
> if (*cp == LDELIM)
> s = cp;
> }
>
> If the first test triggers but it doesn't then find a following
> paren, then it's incremented depth without moving s, which seems
> certain to end badly. Perhaps the correct fix is like
>
> if (*s == LDELIM_C)
> depth++, s++;
> else if (*s == LDELIM)
> {
> /* If there are two left parens, consume the first one */
> cp = (s + 1);
> while (isspace((unsigned char) *cp))
> cp++;
> if (*cp == LDELIM)
> depth++, s = cp;
> }
>
> regards, tom lane
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
