[TRAFODION-2725] SQL types are REAL, FLOAT, and DOUBLE. Some values are inserted, a stack overflow occurs when SQLGetData is executed.

[SuJinpei]

root cause: write stack variable out of range.
unsigned long ODBC::ConvertSQLToC(...., targetLength) {
...
CHAR cTmpBuf[132];
...
double_to_char (dTmp, FLT_DIG + 1, cTmpBuf, targetLength) // when targetLength > 132, the stack will be broken.
...
}
fixed by: limit the max size to sizeof(cTmpBuf)
test result: run coast to check, no new issue introduced.

[Traf-Jenkins]

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2028/

[Traf-Jenkins]

Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2028/

[DaveBirdsall]

+1 Looks good to me

[selvaganesang]

While reviewing the double_to_char function to determine if the size includes null termination, I found that ecvt function is being used to convert the double to character. ecvt function is not thread safe. Can you please fix the surround code too.

Also, it looks like size shouldn't include null termination. The refactored code using sprintf can decide how the size should be used

[SuJinpei]

Hi Selva, thank you for your comments. seems ecvt function is obsolete, sprintf is recommended. Can I use sprintf to refactor the function double_to_char?

[selvaganesang]

Yes. you can using sprintf

[Traf-Jenkins]

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2049/

[Traf-Jenkins]

Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2049/

[Traf-Jenkins]

Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2050/

[Traf-Jenkins]

Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2050/

[Traf-Jenkins]

Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2051/

[Traf-Jenkins]

Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2051/

[ghost]

need initialize. char buf[MAX_DOUBLE_TO_CHAR_LEN] = {0};

[ghost]

snprint is better.

[SuJinpei]

I don't think there is a risk of writing out of range. So I think sprintf is better.

[selvaganesang]

There is a risk of writing out of range because precision is not checked to be within the buf size. Also, you need to allow the numbers to be truncated after a decimal even when there is no sufficient length passed by the caller. So size can be less than strlen(buf) as long as the fraction part alone is getting cut.

[SuJinpei]

@selvaganesang from original code logic, size should larger than strlen(buf). if we still want to return reasonable value though size less than strlen(buf), I think we should use other algorithm instead of snprintf.

[Traf-Jenkins]

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2052/

[Traf-Jenkins]

Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2052/

[Traf-Jenkins]

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2060/

[Traf-Jenkins]

Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2060/

[Traf-Jenkins]

Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2061/

[Traf-Jenkins]

Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2061/

[Traf-Jenkins]

Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2062/

[Traf-Jenkins]

Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2062/

[selvaganesang]

+1. The concepts for this change look good. I am assuming that the boundary conditions are unit tested.

[SuJinpei]

@selvaganesang Yes, I tested some boundary conditions, the result is expected. test program like below:

int main()
{
    char buf[5120];
    bool rc = false;
    int p = DBL_MANT_DIG-DBL_MIN_EXP;
    unsigned long long dsb[] = {
        0x3ff0000000000000,
        0x3ff0000000000001,
        0x0000000000000001,
        0x000fffffffffffff,
        0x001fffffffffffff,
        0x0010000000000000,
        0x7fefffffffffffff,
        0x800fffffffffffff,
        0x8000000000000001
    };
    int precisions[] = {
        0,1,2,4,8,15,DBL_MANT_DIG - DBL_MIN_EXP
    };

    int sizes[] = {
        -1, 0, 1
    };

    double *ds = (double *)&dsb;

    printf("sizeof(dsb) / sizeof(double) = %d\n", sizeof(dsb) / sizeof(double));

    for (size_t i = 0; i < sizeof(dsb) / sizeof(double); ++i) {
        for (size_t j = 0; j < sizeof(precisions) / sizeof(int); ++j) {
            for (size_t k = 0; k < sizeof(sizes) / sizeof(int); ++k) {
                rc = double_to_char(ds[i], precisions[j], buf, precisions[j] + sizes[k]);
                printf(rc == true ? "number=%lg, precision=%d, size=%d, buf=%s, strlen(buf)=%d\n" : "number=%lg, precision=%d, size=%d, return false\n",
                    ds[i], precisions[j], precisions[j] + sizes[k], buf, strlen(buf));
            }
        }
    }

    return 0;
}

[DaveBirdsall]

Looks like this is ready to merge. I will merge it.