Conversion of text with '\0' to DECFLOAT without errors.

[dmitry-lipetsk]

SQL> show version;
ISQL Version: WI-V4.0.3.2937 Firebird 4.0
Server version:
Firebird/Windows/AMD/Intel/x64 (access method), version "WI-V4.0.3.2937 Firebird 4.0"
Firebird/Windows/AMD/Intel/x64 (remote server), version "WI-V4.0.3.2937 Firebird 4.0/tcp (HOME4)/P17:C"
Firebird/Windows/AMD/Intel/x64 (remote interface), version "WI-V4.0.3.2937 Firebird 4.0/tcp (HOME4)/P17:C"
on disk structure version 13.0
SQL> select cast('123'||x'00'||'321' as integer) from rdb$database;

        CAST
============
Statement failed, SQLSTATE = 22018
conversion error from string "123"
SQL> select cast('123'||x'00'||'321' as DECFLOAT(34)) from rdb$database;

                                      CAST
==========================================
                                       123

[AlexPeshkoff]

BTW diags in case of integer is also rather far from ideal

[dmitry-lipetsk]

Alex, I rewrote decFloatFromString to work with a range of characters, and I seem to have found a few pointer issues.

If you haven't started dealing with this issue yet, you can wait while I finish and provide more information.

[AlexPeshkoff]

Have not. It will be great. BTW, if there are issues with decFloatFromString, that probably should be reported to IBM.

[dmitry-lipetsk]

About a problem with pointers. I'll try to explian a problem as simple as possible :)

Sample query:

select cast('-0000000000000000000000000000000000000000000000000000000000000000000000000000000.E+7000' as DECFLOAT(34))
from rdb$database

decFinalize function receives 'num' with this data:

As you can see, lsd<msd and both members are pointing to uninitialized memory.

msd is pointing on MOST significant digit lsd is pointing LEAST significant digit

decFinalize expectes that num contains at least one digit - it means that msd<=lsd

Look at this code of decFinalize:

https://github.com/FirebirdSQL/firebird/blob/5ed9d76d2951501e0e5b344ef45c064e0da0131e/extern/decNumber/decCommon.c#L261-L269

It reads this bad data in line 265 without any preliminary checks (NUMISSPECIAL does not check these pointers).

This situation is occurring by mistake in decFloatFromString

https://github.com/FirebirdSQL/firebird/blob/5ed9d76d2951501e0e5b344ef45c064e0da0131e/extern/decNumber/decCommon.c#L906-L917

This cycle must keep at least one (the last) zero symbol but it is ignoring all them. The reason for it - the last '.'

I rewrote this code:

        // [This is a rare and unusual case; arbitrary-length input]
        // strip leading zeros [but leave final 0 if all 0's]
        {
          char* cfirst2=NULL;

          for (; cfirst<=clast; cfirst++) {
              if (*cfirst == '.') continue;  // [ignore]
              cfirst2=cfirst;                // let's save the position of this digit
              if (*cfirst!='0') break;       // done
              assert(digits>0);
              digits--;                      // 0 stripped
            } // cfirst

          assert(cfirst2 != NULL);
          assert(cfirst2 <= clast);

          if(clast<cfirst) { // all the symbols are ZEROs
            assert(digits==0);
            digits=1;
            } else {
                assert(digits>0);
              }
          cfirst=cfirst2;
        } // local - at least one leading 0

Now decFinalize receives the correct presentation of 'num':

I ran my others tests - it seems all is ok, too.

[dmitry-lipetsk]

The second problem with pointers lives in decFinalize function and I am not sure that I am right.

Look at this code:

https://github.com/FirebirdSQL/firebird/blob/5ed9d76d2951501e0e5b344ef45c064e0da0131e/extern/decNumber/decCommon.c#L455-L469

Lines with problem: https://github.com/FirebirdSQL/firebird/blob/5ed9d76d2951501e0e5b344ef45c064e0da0131e/extern/decNumber/decCommon.c#L467-L468

buffer it is a local memory and can be freed/reused after this block.

I think this 'buffer' must be declared at the top level of decFinalize.

[AlexPeshkoff]

Confirm both problems - with pointers and illegally located buffer. Can you prepare PR?

[dmitry-lipetsk]

Yes, no problems. For FB4 and HEAD.

[dmitry-lipetsk]

Can I add asserts (native asserts, not fb_asserts)?

[AlexPeshkoff]

If asserts are already used in original decfloat library code - yes, certainly. If not - I think it's better to avoid them in order to follow code style of the project. Provided you are not going to rework all code - hope not :)

[dmitry-lipetsk]

Alex, applyed PR does not fix this problem :)

For fix of this issue it is need

1. Detect '\0' before the call of fromString function or
2. Create a new fromString function that will work with range of symbols

[AlexPeshkoff]

On 5/30/23 14:39, Dmitry Kovalenko wrote:

Detect '\0' before the call of fromString function

That's what I was going to do initially... Will reopen an issue.

[AlexPeshkoff]

Also raise conversion error instead indeterminant decfloat error and output symbols < 32 in \xDD format. Single \ replaced with \.

[dmitry-lipetsk]

Also raise conversion error instead indeterminant decfloat error and output symbols < 32 in \xDD format. Single \ replaced with .

I think decFloatFromString_fb can be upgraded to return an additional info about a bad position and this information can be used for creating a detail message.

At the next stage, of course, not right now.

[AlexPeshkoff]

That makes sense provided all conversion errors are accompanied with error column info.