Potential issue in ecpg-informix decimal converting functions

Started by Nonamealmost 2 years ago12 messages
#1Noname
a.imamov@postgrespro.ru
1 attachment(s)

Hi, everyone!

I found a potential bug in dectoint() and dectolong() functions from
informix.c. "Informix Compatibility Mode" doc chapter says that
ECPG_INFORMIX_NUM_OVERFLOW is returned if an overflow occurred. But
check this line in dectoint() or dectolong() (it is present in both):
if (ret == PGTYPES_NUM_OVERFLOW) - condition is always
false because PGTYPESnumeric_to_int() and PGTYPESnumeric_to_long()
functions return only 0 or -1. So ECPG_INFORMIX_NUM_OVERFLOW can never
be returned.

I think dectoint(), dectolong() and PGTYPESnumeric_to_int() functions
should be a little bit different like in proposing patch.
What do you think?

The flaw was catched with the help of Svace static analyzer.
https://svace.pages.ispras.ru/svace-website/en/

Thank you!

Attachments:

informix_convert_from_decimal.patchtext/x-diff; name=informix_convert_from_decimal.patchDownload
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 73351a9136..8390a61cf4 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -8882,7 +8882,7 @@ int dectodbl(decimal *np, double *dblp);
       <term><function>dectoint</function></term>
       <listitem>
        <para>
-        Convert a variable to type decimal to an integer.
+        Convert a variable of type decimal to an integer.
 <synopsis>
 int dectoint(decimal *np, int *ip);
 </synopsis>
@@ -8892,8 +8892,9 @@ int dectoint(decimal *np, int *ip);
        </para>
        <para>
         On success, 0 is returned and a negative value if the conversion
-        failed. If an overflow occurred, <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal>
-        is returned.
+        failed. If overflow or underflow occurred, the function returns
+        <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal> or
+        <literal>ECPG_INFORMIX_NUM_UNDERFLOW</literal> respectively.
        </para>
        <para>
         Note that the ECPG implementation differs from the <productname>Informix</productname>
@@ -8908,7 +8909,7 @@ int dectoint(decimal *np, int *ip);
       <term><function>dectolong</function></term>
       <listitem>
        <para>
-        Convert a variable to type decimal to a long integer.
+        Convert a variable of type decimal to a long integer.
 <synopsis>
 int dectolong(decimal *np, long *lngp);
 </synopsis>
@@ -8918,8 +8919,9 @@ int dectolong(decimal *np, long *lngp);
        </para>
        <para>
         On success, 0 is returned and a negative value if the conversion
-        failed. If an overflow occurred, <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal>
-        is returned.
+        failed. If overflow or underflow occurred, the function returns
+        <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal> or
+        <literal>ECPG_INFORMIX_NUM_UNDERFLOW</literal> respectively.
        </para>
        <para>
         Note that the ECPG implementation differs from the <productname>Informix</productname>
diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
index 80d40aa3e0..73e84bc6e3 100644
--- a/src/interfaces/ecpg/compatlib/informix.c
+++ b/src/interfaces/ecpg/compatlib/informix.c
@@ -435,6 +435,7 @@ dectoint(decimal *np, int *ip)
 {
 	int			ret;
 	numeric    *nres = PGTYPESnumeric_new();
+	int			errnum = 0;
 
 	if (nres == NULL)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
@@ -445,11 +446,18 @@ dectoint(decimal *np, int *ip)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
 	}
 
+	errno = 0;
 	ret = PGTYPESnumeric_to_int(nres, ip);
+	errnum = errno;
 	PGTYPESnumeric_free(nres);
 
-	if (ret == PGTYPES_NUM_OVERFLOW)
-		ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	if (ret)
+	{
+		if (errnum == PGTYPES_NUM_UNDERFLOW)
+			ret = ECPG_INFORMIX_NUM_UNDERFLOW;
+		else if (errnum == PGTYPES_NUM_OVERFLOW)
+			ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	}
 
 	return ret;
 }
@@ -459,6 +467,7 @@ dectolong(decimal *np, long *lngp)
 {
 	int			ret;
 	numeric    *nres = PGTYPESnumeric_new();
+	int			errnum = 0;
 
 	if (nres == NULL)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
@@ -469,11 +478,18 @@ dectolong(decimal *np, long *lngp)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
 	}
 
+	errno = 0;
 	ret = PGTYPESnumeric_to_long(nres, lngp);
+	errnum = errno;
 	PGTYPESnumeric_free(nres);
 
-	if (ret == PGTYPES_NUM_OVERFLOW)
-		ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	if (ret)
+	{
+		if (errnum == PGTYPES_NUM_UNDERFLOW)
+			ret = ECPG_INFORMIX_NUM_UNDERFLOW;
+		else if (errnum == PGTYPES_NUM_OVERFLOW)
+			ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	}
 
 	return ret;
 }
diff --git a/src/interfaces/ecpg/pgtypeslib/numeric.c b/src/interfaces/ecpg/pgtypeslib/numeric.c
index 35e7b92da4..52b49e1ce9 100644
--- a/src/interfaces/ecpg/pgtypeslib/numeric.c
+++ b/src/interfaces/ecpg/pgtypeslib/numeric.c
@@ -1502,7 +1502,12 @@ PGTYPESnumeric_to_int(numeric *nv, int *ip)
 /* silence compilers that might complain about useless tests */
 #if SIZEOF_LONG > SIZEOF_INT
 
-	if (l < INT_MIN || l > INT_MAX)
+	if (l < INT_MIN)
+	{
+		errno = PGTYPES_NUM_UNDERFLOW;
+		return -1;
+	}
+	else if(l > INT_MAX)
 	{
 		errno = PGTYPES_NUM_OVERFLOW;
 		return -1;
diff --git a/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout b/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
index 1f8675b3f3..71a5afa4a7 100644
--- a/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
+++ b/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
@@ -3,8 +3,8 @@
 (no errno set) - dec[0,3]: r: -1, *
 (no errno set) - dec[0,4]: r: -1, *
 dec[0,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[0,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[0,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[0,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[0,8]: 0 (r: -1200)
 (errno == PGTYPES_NUM_OVERFLOW) - dec[0,10]: 0 (r: -1)
 
 dec[1,1]: r: 0, -2
@@ -45,8 +45,8 @@ dec[4,2]: r: 0, 592490000000000000000000
 dec[4,3]: r: 0, 592490000000000000000000.0
 dec[4,4]: r: 0, 592490000000000000000000.00
 dec[4,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[4,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[4,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[4,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[4,8]: 0 (r: -1200)
 dec[4,10]: 5.9249e+23 (r: 0)
 
 dec[5,1]: r: 0, -328400
@@ -141,8 +141,8 @@ dec[13,2]: r: 0, 1234567890123456789012345679
 dec[13,3]: r: 0, 1234567890123456789012345678.9
 dec[13,4]: r: 0, 1234567890123456789012345678.91
 dec[13,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[13,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[13,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[13,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[13,8]: 0 (r: -1200)
 dec[13,10]: 1.23457e+27 (r: 0)
 
 (errno == PGTYPES_NUM_OVERFLOW) - dec[14,0]: r: -1200
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Noname (#1)
Re: Potential issue in ecpg-informix decimal converting functions

On 22 Feb 2024, at 17:54, a.imamov@postgrespro.ru wrote:

PGTYPESnumeric_to_int() and PGTYPESnumeric_to_long()
functions return only 0 or -1. So ECPG_INFORMIX_NUM_OVERFLOW can never
be returned.

Indeed, this looks like an oversight.

I think dectoint(), dectolong() and PGTYPESnumeric_to_int() functions
should be a little bit different like in proposing patch.
What do you think?

-        Convert a variable to type decimal to an integer.
+        Convert a variable of type decimal to an integer.
While related, this should be committed and backpatched regardless.

+ int errnum = 0;
Stylistic nit, we typically don't initialize a variable which cannot be
accessed before being set.

Overall the patch looks sane, please register it for the next commitfest to
make it's not missed.

--
Daniel Gustafsson

#3Noname
a.imamov@postgrespro.ru
In reply to: Daniel Gustafsson (#2)
1 attachment(s)
Re: Potential issue in ecpg-informix decimal converting functions

Daniel Gustafsson писал(а) 2024-02-23 13:44:

On 22 Feb 2024, at 17:54, a.imamov@postgrespro.ru wrote:

PGTYPESnumeric_to_int() and PGTYPESnumeric_to_long()
functions return only 0 or -1. So ECPG_INFORMIX_NUM_OVERFLOW can never
be returned.

Indeed, this looks like an oversight.

I think dectoint(), dectolong() and PGTYPESnumeric_to_int() functions
should be a little bit different like in proposing patch.
What do you think?

-        Convert a variable to type decimal to an integer.
+        Convert a variable of type decimal to an integer.
While related, this should be committed and backpatched regardless.

+ int errnum = 0;
Stylistic nit, we typically don't initialize a variable which cannot be
accessed before being set.

Overall the patch looks sane, please register it for the next
commitfest to
make it's not missed.

--
Daniel Gustafsson

Thank you for feedback,

-        Convert a variable to type decimal to an integer.
+        Convert a variable of type decimal to an integer.
I removed this from the patch and proposed to 
pgsql-docs@lists.postgresql.org

+ int errnum = 0;
fixed

Thank's for advice, the patch will be registered for the next
commitfest.

--
Aidar Imamov

Attachments:

informix_convert_from_decimal.patchtext/x-diff; name=informix_convert_from_decimal.patchDownload
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 73351a9136..aa86afa1d9 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -8892,8 +8892,9 @@ int dectoint(decimal *np, int *ip);
        </para>
        <para>
         On success, 0 is returned and a negative value if the conversion
-        failed. If an overflow occurred, <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal>
-        is returned.
+        failed. If overflow or underflow occurred, the function returns
+        <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal> or
+        <literal>ECPG_INFORMIX_NUM_UNDERFLOW</literal> respectively.
        </para>
        <para>
         Note that the ECPG implementation differs from the <productname>Informix</productname>
@@ -8918,8 +8919,9 @@ int dectolong(decimal *np, long *lngp);
        </para>
        <para>
         On success, 0 is returned and a negative value if the conversion
-        failed. If an overflow occurred, <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal>
-        is returned.
+        failed. If overflow or underflow occurred, the function returns
+        <literal>ECPG_INFORMIX_NUM_OVERFLOW</literal> or
+        <literal>ECPG_INFORMIX_NUM_UNDERFLOW</literal> respectively.
        </para>
        <para>
         Note that the ECPG implementation differs from the <productname>Informix</productname>
diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
index 80d40aa3e0..5f9d71d0f3 100644
--- a/src/interfaces/ecpg/compatlib/informix.c
+++ b/src/interfaces/ecpg/compatlib/informix.c
@@ -434,6 +434,7 @@ int
 dectoint(decimal *np, int *ip)
 {
 	int			ret;
+	int			errnum;
 	numeric    *nres = PGTYPESnumeric_new();
 
 	if (nres == NULL)
@@ -445,11 +446,18 @@ dectoint(decimal *np, int *ip)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
 	}
 
+	errno = 0;
 	ret = PGTYPESnumeric_to_int(nres, ip);
+	errnum = errno;
 	PGTYPESnumeric_free(nres);
 
-	if (ret == PGTYPES_NUM_OVERFLOW)
-		ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	if (ret)
+	{
+		if (errnum == PGTYPES_NUM_UNDERFLOW)
+			ret = ECPG_INFORMIX_NUM_UNDERFLOW;
+		else if (errnum == PGTYPES_NUM_OVERFLOW)
+			ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	}
 
 	return ret;
 }
@@ -458,6 +466,7 @@ int
 dectolong(decimal *np, long *lngp)
 {
 	int			ret;
+	int			errnum;
 	numeric    *nres = PGTYPESnumeric_new();
 
 	if (nres == NULL)
@@ -469,11 +478,18 @@ dectolong(decimal *np, long *lngp)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
 	}
 
+	errno = 0;
 	ret = PGTYPESnumeric_to_long(nres, lngp);
+	errnum = errno;
 	PGTYPESnumeric_free(nres);
 
-	if (ret == PGTYPES_NUM_OVERFLOW)
-		ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	if (ret)
+	{
+		if (errnum == PGTYPES_NUM_UNDERFLOW)
+			ret = ECPG_INFORMIX_NUM_UNDERFLOW;
+		else if (errnum == PGTYPES_NUM_OVERFLOW)
+			ret = ECPG_INFORMIX_NUM_OVERFLOW;
+	}
 
 	return ret;
 }
diff --git a/src/interfaces/ecpg/pgtypeslib/numeric.c b/src/interfaces/ecpg/pgtypeslib/numeric.c
index 35e7b92da4..52b49e1ce9 100644
--- a/src/interfaces/ecpg/pgtypeslib/numeric.c
+++ b/src/interfaces/ecpg/pgtypeslib/numeric.c
@@ -1502,7 +1502,12 @@ PGTYPESnumeric_to_int(numeric *nv, int *ip)
 /* silence compilers that might complain about useless tests */
 #if SIZEOF_LONG > SIZEOF_INT
 
-	if (l < INT_MIN || l > INT_MAX)
+	if (l < INT_MIN)
+	{
+		errno = PGTYPES_NUM_UNDERFLOW;
+		return -1;
+	}
+	else if(l > INT_MAX)
 	{
 		errno = PGTYPES_NUM_OVERFLOW;
 		return -1;
diff --git a/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout b/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
index 1f8675b3f3..71a5afa4a7 100644
--- a/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
+++ b/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
@@ -3,8 +3,8 @@
 (no errno set) - dec[0,3]: r: -1, *
 (no errno set) - dec[0,4]: r: -1, *
 dec[0,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[0,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[0,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[0,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[0,8]: 0 (r: -1200)
 (errno == PGTYPES_NUM_OVERFLOW) - dec[0,10]: 0 (r: -1)
 
 dec[1,1]: r: 0, -2
@@ -45,8 +45,8 @@ dec[4,2]: r: 0, 592490000000000000000000
 dec[4,3]: r: 0, 592490000000000000000000.0
 dec[4,4]: r: 0, 592490000000000000000000.00
 dec[4,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[4,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[4,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[4,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[4,8]: 0 (r: -1200)
 dec[4,10]: 5.9249e+23 (r: 0)
 
 dec[5,1]: r: 0, -328400
@@ -141,8 +141,8 @@ dec[13,2]: r: 0, 1234567890123456789012345679
 dec[13,3]: r: 0, 1234567890123456789012345678.9
 dec[13,4]: r: 0, 1234567890123456789012345678.91
 dec[13,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[13,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[13,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[13,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[13,8]: 0 (r: -1200)
 dec[13,10]: 1.23457e+27 (r: 0)
 
 (errno == PGTYPES_NUM_OVERFLOW) - dec[14,0]: r: -1200
#4Michael Paquier
michael@paquier.xyz
In reply to: Noname (#3)
Re: Potential issue in ecpg-informix decimal converting functions

On Fri, Feb 23, 2024 at 06:03:41PM +0300, a.imamov@postgrespro.ru wrote:

Thank's for advice, the patch will be registered for the next commitfest.

The risk looks really minimal to me, but playing with error codes
while the logic of the function is unchanged does not strike me as
something to backpatch as it could slightly break applications. On
HEAD, I'm OK with that.
--
Michael

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#4)
Re: Potential issue in ecpg-informix decimal converting functions

On 24 Feb 2024, at 02:15, Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Feb 23, 2024 at 06:03:41PM +0300, a.imamov@postgrespro.ru wrote:

Thank's for advice, the patch will be registered for the next commitfest.

The risk looks really minimal to me, but playing with error codes
while the logic of the function is unchanged does not strike me as
something to backpatch as it could slightly break applications. On
HEAD, I'm OK with that.

Yeah, I think this is for HEAD only, especially given the lack of complaints
against backbranches.

--
Daniel Gustafsson

#6Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#5)
Re: Potential issue in ecpg-informix decimal converting functions

On Mon, Feb 26, 2024 at 12:28:51AM +0100, Daniel Gustafsson wrote:

Yeah, I think this is for HEAD only, especially given the lack of complaints
against backbranches.

Daniel, are you planning to look at that? I haven't done any detailed
lookup, but would be happy to do so it that helps.
--
Michael

#7Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#6)
Re: Potential issue in ecpg-informix decimal converting functions

On 27 Feb 2024, at 06:08, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Feb 26, 2024 at 12:28:51AM +0100, Daniel Gustafsson wrote:

Yeah, I think this is for HEAD only, especially given the lack of complaints
against backbranches.

Daniel, are you planning to look at that? I haven't done any detailed
lookup, but would be happy to do so it that helps.

I have it on my TODO for the upcoming CF.

--
Daniel Gustafsson

#8Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#7)
Re: Potential issue in ecpg-informix decimal converting functions

On Tue, Feb 27, 2024 at 09:24:25AM +0100, Daniel Gustafsson wrote:

I have it on my TODO for the upcoming CF.

Okay, thanks.
--
Michael

#9Noname
a.imamov@postgrespro.ru
In reply to: Michael Paquier (#8)
Re: Potential issue in ecpg-informix decimal converting functions

Michael Paquier писал(а) 2024-02-28 02:14:

On Tue, Feb 27, 2024 at 09:24:25AM +0100, Daniel Gustafsson wrote:

I have it on my TODO for the upcoming CF.

Okay, thanks.
--
Michael

Greetings!

Sorry, I had been waiting for a few days for my cool-off period to end.
The patch now is registered to CF in the 'Refactoring' topic.

--
Aidar

#10Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#6)
1 attachment(s)
Re: Potential issue in ecpg-informix decimal converting functions

On 27 Feb 2024, at 06:08, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Feb 26, 2024 at 12:28:51AM +0100, Daniel Gustafsson wrote:

Yeah, I think this is for HEAD only, especially given the lack of complaints
against backbranches.

Daniel, are you planning to look at that? I haven't done any detailed
lookup, but would be happy to do so it that helps.

I had a look at this today and opted for trimming back the patch a bit.
Reading the informix docs the functions we are mimicking for compatibility here
does not have an underflow returnvalue, so adding one doesn't seem right (or
helpful). The attached fixes the return of overflow and leaves it at that,
which makes it possible to backpatch since it's fixing the code to match the
documented behavior.

--
Daniel Gustafsson

Attachments:

v3-0001-ecpg-Fix-return-code-for-overflow-in-numeric-conv.patchapplication/octet-stream; name=v3-0001-ecpg-Fix-return-code-for-overflow-in-numeric-conv.patch; x-unix-mode=0644Download
From 4accb63b937357c06271e7b137dc9a6fe3baff01 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Wed, 6 Mar 2024 15:42:26 +0100
Subject: [PATCH v3] ecpg: Fix return code for overflow in numeric conversion

The decimal conversion functions dectoint and dectolong are documented
to return ECPG_INFORMIX_NUM_OVERFLOW in case of overflows, but always
returned -1 on all errors due to incorrectly checking the returnvalue
from the PGTYPES* functions.

Author: Aidar Imamov <a.imamov@postgrespro.ru>
Discussion: https://postgr.es/m/54d2b53327516d9454daa5fb2f893bdc@postgrespro.ru
---
 src/interfaces/ecpg/compatlib/informix.c             | 10 ++++++++--
 .../test/expected/compat_informix-dec_test.stdout    | 12 ++++++------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
index 80d40aa3e0..8ea89e640a 100644
--- a/src/interfaces/ecpg/compatlib/informix.c
+++ b/src/interfaces/ecpg/compatlib/informix.c
@@ -435,6 +435,7 @@ dectoint(decimal *np, int *ip)
 {
 	int			ret;
 	numeric    *nres = PGTYPESnumeric_new();
+	int			errnum;
 
 	if (nres == NULL)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
@@ -445,10 +446,12 @@ dectoint(decimal *np, int *ip)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
 	}
 
+	errno = 0;
 	ret = PGTYPESnumeric_to_int(nres, ip);
+	errnum = errno;
 	PGTYPESnumeric_free(nres);
 
-	if (ret == PGTYPES_NUM_OVERFLOW)
+	if (ret == -1 && errnum == PGTYPES_NUM_OVERFLOW)
 		ret = ECPG_INFORMIX_NUM_OVERFLOW;
 
 	return ret;
@@ -459,6 +462,7 @@ dectolong(decimal *np, long *lngp)
 {
 	int			ret;
 	numeric    *nres = PGTYPESnumeric_new();
+	int			errnum;
 
 	if (nres == NULL)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
@@ -469,10 +473,12 @@ dectolong(decimal *np, long *lngp)
 		return ECPG_INFORMIX_OUT_OF_MEMORY;
 	}
 
+	errno = 0;
 	ret = PGTYPESnumeric_to_long(nres, lngp);
+	errnum = errno;
 	PGTYPESnumeric_free(nres);
 
-	if (ret == PGTYPES_NUM_OVERFLOW)
+	if (ret == -1 && errnum == PGTYPES_NUM_OVERFLOW)
 		ret = ECPG_INFORMIX_NUM_OVERFLOW;
 
 	return ret;
diff --git a/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout b/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
index 1f8675b3f3..71a5afa4a7 100644
--- a/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
+++ b/src/interfaces/ecpg/test/expected/compat_informix-dec_test.stdout
@@ -3,8 +3,8 @@
 (no errno set) - dec[0,3]: r: -1, *
 (no errno set) - dec[0,4]: r: -1, *
 dec[0,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[0,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[0,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[0,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[0,8]: 0 (r: -1200)
 (errno == PGTYPES_NUM_OVERFLOW) - dec[0,10]: 0 (r: -1)
 
 dec[1,1]: r: 0, -2
@@ -45,8 +45,8 @@ dec[4,2]: r: 0, 592490000000000000000000
 dec[4,3]: r: 0, 592490000000000000000000.0
 dec[4,4]: r: 0, 592490000000000000000000.00
 dec[4,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[4,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[4,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[4,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[4,8]: 0 (r: -1200)
 dec[4,10]: 5.9249e+23 (r: 0)
 
 dec[5,1]: r: 0, -328400
@@ -141,8 +141,8 @@ dec[13,2]: r: 0, 1234567890123456789012345679
 dec[13,3]: r: 0, 1234567890123456789012345678.9
 dec[13,4]: r: 0, 1234567890123456789012345678.91
 dec[13,5]: r: 0, 0.00
-(errno == PGTYPES_NUM_OVERFLOW) - dec[13,6]: 0 (r: -1)
-(errno == PGTYPES_NUM_OVERFLOW) - dec[13,8]: 0 (r: -1)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[13,6]: 0 (r: -1200)
+(errno == PGTYPES_NUM_OVERFLOW) - dec[13,8]: 0 (r: -1200)
 dec[13,10]: 1.23457e+27 (r: 0)
 
 (errno == PGTYPES_NUM_OVERFLOW) - dec[14,0]: r: -1200
-- 
2.32.1 (Apple Git-133)

#11Noname
a.imamov@postgrespro.ru
In reply to: Daniel Gustafsson (#10)
Re: Potential issue in ecpg-informix decimal converting functions

Daniel Gustafsson писал(а) 2024-03-06 18:03:

On 27 Feb 2024, at 06:08, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Feb 26, 2024 at 12:28:51AM +0100, Daniel Gustafsson wrote:

Yeah, I think this is for HEAD only, especially given the lack of
complaints
against backbranches.

Daniel, are you planning to look at that? I haven't done any detailed
lookup, but would be happy to do so it that helps.

I had a look at this today and opted for trimming back the patch a bit.
Reading the informix docs the functions we are mimicking for
compatibility here
does not have an underflow returnvalue, so adding one doesn't seem
right (or
helpful). The attached fixes the return of overflow and leaves it at
that,
which makes it possible to backpatch since it's fixing the code to
match the
documented behavior.

--
Daniel Gustafsson

I agree with the proposed changes in favor of backward compatibility.
Also, is it a big deal that the PGTYPESnumeric_to_long() function
doesn't
exactly match the documentation, compared to PGTYPESnumeric_to_int()? It
handles underflow case separately and sets errno to
PGTYPES_NUM_UNDERFLOW
additionally.

#12Daniel Gustafsson
daniel@yesql.se
In reply to: Noname (#11)
Re: Potential issue in ecpg-informix decimal converting functions

On 6 Mar 2024, at 20:12, a.imamov@postgrespro.ru wrote:

I agree with the proposed changes in favor of backward compatibility.

I went ahead to pushed this after another look. I'm a bit hesitant to
backpatch this since there are no reports against it, and I don't have good
sense for how ECPG code is tested and maintained across minor version upgrades.
If we want to I will of course do so, so please chime in in case there are
different and more informed opinions.

Also, is it a big deal that the PGTYPESnumeric_to_long() function doesn't
exactly match the documentation, compared to PGTYPESnumeric_to_int()? It
handles underflow case separately and sets errno to PGTYPES_NUM_UNDERFLOW
additionally.

Fixed as well.

--
Daniel Gustafsson