Re: Fix inappropriate uses of atol()

On 03/08/2024 18:20, Tom Lane wrote:
> Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
>> On 03/08/2024 14:04, Peter Eisentraut wrote:
>>> I noticed (during [0]) to some uses of the function atol() seem
>>> inappropriate.
>
>> +1 except for this one:
>
>>> /* If we have just one character this is not a string */
>>> - if (atol(p->type->size) == 1)
>>> + if (atoi(p->type->size) == 1)
>>> mmerror(PARSE_ERROR, ET_ERROR, "invalid data type");
>
> How about
>
> - if (atol(p->type->size) == 1)
> + if (strcmp(p->type->size, "1") == 0)
>
> ? I've not actually tested, but this should catch the cases the
> warning is meant to catch while not complaining about any of the
> examples you give.

Makes sense.

> I'm not sure if leading/trailing spaces
> would fool it (i.e., "char foo[ 1 ];"). But even if they do,
> that doesn't seem disastrous.
Right. There are many ways to fool it in that direction, e.g.

#define ONE 1
char foo[ONE];

I'm actually not even sure if it's intentional to throw the error even
with "char[1]". It makes sense to give an error on "char", but who says
that "char[1]" isn't a valid string? Not a very useful string, because
it can only hold the empty string, but a string nevertheless, and
sometimes an empty string is exactly what you want.

If we can easily distinguish between "char" and any array of char here,
might be best to accept the all arrays regardless of the length.

--
Heikki Linnakangas
Neon (https://neon.tech)