public inbox for [email protected]  

help / color / mirror / Atom feed

From: Heikki Linnakangas <[email protected]>
To: Tom Lane <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Fix inappropriate uses of atol()
Date: Sat, 3 Aug 2024 18:27:00 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>

On 03/08/2024 18:20, Tom Lane wrote:
> Heikki Linnakangas <[email protected]> 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)

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Fix inappropriate uses of atol()
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox