* Bug 16508: csvTextScan did not handle well complex data. 64/21564/5
Adeline CARNIS [Fri, 31 Jul 2020 13:21:58 +0000 (15:21 +0200)]
An invalid read had also been fixed for some tests. All spreadsheet
tests pass without any ASAN issue.

https://bugzilla.scilab.org/show_bug.cgi?id=16508

Change-Id: I2db157ce71852d914f984460ec0049e697e132d1

scilab/CHANGES.md
scilab/modules/spreadsheet/src/c/csvRead.c
scilab/modules/spreadsheet/tests/nonreg_tests/bug_16508.dia.ref [new file with mode: 0644]
scilab/modules/spreadsheet/tests/nonreg_tests/bug_16508.tst [new file with mode: 0644]
scilab/modules/string/src/c/stringToComplex.c

index b1c467b..cf3ce0a 100644 (file)
@@ -332,6 +332,7 @@ Bug Fixes
 * [#16474](https://bugzilla.scilab.org/16474): `imult(%z)` crashed Scilab.
 * [#16488](https://bugzilla.scilab.org/16488): Concatenations mixing boolean and double with at least one operand being sparse were not supported.
 * [#16496](https://bugzilla.scilab.org/16496): The `getdate` page should be rewritten: a) `getdate("s")` does NOT take leap seconds into account. b) `D=getdate(X)` is vectorized, accepts fractional seconds and returns them in `[0,1)` in D(10) instead of milliseconds. Moreover, the time referential of the result was unclear (time zone, daylight saving offset).
+* [#16508](https://bugzilla.scilab.org/16508): csvTextScan and csvRead did not handle well complex data.
 * [#16512](https://bugzilla.scilab.org/16512): 1 ./ uint8(0) crashes Scilab (idem with int8, uint16, int16, uint32, int32, uint64, int64).
 * [#16517](https://bugzilla.scilab.org/16517): `getdate("s")` truncated the actual time to integer seconds. `getdate(u)(10)` returned fractional seconds instead of milliseconds as `getdate()`.
 * [#16522](https://bugzilla.scilab.org/16522): `bitget(x,pos)` and `bitset(x,pos)` results could be wrong when `pos` is an encoded integer.
index 1fd1887..962b55a 100644 (file)
@@ -231,7 +231,7 @@ int csvTextScanInPlace(wchar_t** text, int nbLines, const wchar_t* separator,
             if (pDblRealValues != NULL)
             {
                 doublecomplex v = stringToComplexWInPlace(start, end, decimal,
-                                                          TRUE, &ierr);
+                                  TRUE, &ierr);
                 if (ierr == STRINGTOCOMPLEX_NO_ERROR)
                 {
                     // the imag part of a complex number is allocated on demand
@@ -491,7 +491,7 @@ static void moveEmptyLinesToEnd(wchar_t** lines, int* nonEmptyLines)
 {
     if (lines)
     {
-        // move the blank lines at the end
+        // move the blank lines at the end and free them
         int last = *nonEmptyLines - 1;
         for (int i = last; i >= 0; i--)
         {
@@ -499,7 +499,7 @@ static void moveEmptyLinesToEnd(wchar_t** lines, int* nonEmptyLines)
             {
                 // swap
                 wchar_t* str = lines[i];
-                memmove(&lines[i], &lines[i+1], (last - i + 1) * sizeof(wchar_t*));
+                memmove(&lines[i], &lines[i + 1], (last - i) * sizeof(wchar_t*));
                 lines[last] = str;
                 last--;
             }
@@ -513,7 +513,7 @@ static int hasOnlyBlankCharacters(wchar_t** line)
 {
     const wchar_t* iter = *line;
 
-    // EMPTY_STR is a tagged, non-allocated value for the empty string. It is 
+    // EMPTY_STR is a tagged, non-allocated value for the empty string. It is
     // used to flag a blank string.
     if (*line == EMPTY_STR)
     {
diff --git a/scilab/modules/spreadsheet/tests/nonreg_tests/bug_16508.dia.ref b/scilab/modules/spreadsheet/tests/nonreg_tests/bug_16508.dia.ref
new file mode 100644 (file)
index 0000000..373f2e3
--- /dev/null
@@ -0,0 +1,37 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2020 - ESI Group - Adeline CARNIS
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- CLI SHELL MODE -->
+//
+// <-- Non-regression test for bug 16508 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/show_bug.cgi?id=16508
+//
+// <-- Short Description -->
+// csvTextScan and csvRead did not handle well complex data.
+text = ["Inf" "1-i" "i" "Nan-i"];
+expected = [%inf; 1-%i; %i; %nan-%i];
+M = csvTextScan(text);
+assert_checkequal(M, expected);
+File = TMPDIR + "/bug_16508.txt";
+mputl(text, File);
+M = csvRead(File);
+assert_checkequal(M, expected);
+text =["i" "1*i" "0*i" "2i"];
+expected = [%i; %i; 0; 2*%i];
+M = csvTextScan(text);
+assert_checkequal(M, expected);
+text =["%i" "1*%i" "0*%i" "2%i"];
+expected = [%i; %i; 0; 2*%i];
+M = csvTextScan(text);
+assert_checkequal(M, expected);
+text = ["Infi" "1+Nani"];
+csvTextScan(text)
+ ans  =
+   0. + Infi
+   1. + Nani
diff --git a/scilab/modules/spreadsheet/tests/nonreg_tests/bug_16508.tst b/scilab/modules/spreadsheet/tests/nonreg_tests/bug_16508.tst
new file mode 100644 (file)
index 0000000..6bc4341
--- /dev/null
@@ -0,0 +1,39 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2020 - ESI Group - Adeline CARNIS
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- CLI SHELL MODE -->
+//
+// <-- Non-regression test for bug 16508 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/show_bug.cgi?id=16508
+//
+// <-- Short Description -->
+// csvTextScan and csvRead did not handle well complex data.
+
+text = ["Inf" "1-i" "i" "Nan-i"];
+expected = [%inf; 1-%i; %i; %nan-%i];
+M = csvTextScan(text);
+assert_checkequal(M, expected);
+
+File = TMPDIR + "/bug_16508.txt";
+mputl(text, File);
+M = csvRead(File);
+assert_checkequal(M, expected);
+
+text =["i" "1*i" "0*i" "2i"];
+expected = [%i; %i; 0; 2*%i];
+M = csvTextScan(text);
+assert_checkequal(M, expected);
+
+text =["%i" "1*%i" "0*%i" "2%i"];
+expected = [%i; %i; 0; 2*%i];
+M = csvTextScan(text);
+assert_checkequal(M, expected);
+
+text = ["Infi" "1+Nani"];
+csvTextScan(text)
\ No newline at end of file
index 63a7fd8..f211175 100644 (file)
@@ -19,7 +19,7 @@
 #include "stringToDouble.h"
 #include "strsubst.h"
 #include "numericconstants_interface.h"
- /* ========================================================================== */
+/* ========================================================================== */
 #include <ctype.h>
 #include <math.h>
 #include <stdio.h>
@@ -453,15 +453,6 @@ static int ParseNumberW(const wchar_t* tx)
     int lookahead = 0;
     int len = 0;
 
-    if (tx[len] == L'0')
-    {
-        return lookahead;
-    }
-    if (tx[len] < 0)
-    {
-        return lookahead;
-    }
-
     if ((tx[len] == L'+') || (tx[len] == L'-'))
     {
         len++;
@@ -623,11 +614,11 @@ static stringToComplexError ParseComplexValue(const char* tx, BOOL bConvertByNAN
                     }
                 }
 
-            if (strcmp(inum_string, "+") == 0)
-            {
-                FREE(inum_string);
-                inum_string = os_strdup("+1");
-            }
+                if (strcmp(inum_string, "+") == 0)
+                {
+                    FREE(inum_string);
+                    inum_string = os_strdup("+1");
+                }
 
                 if (strcmp(inum_string, "-") == 0)
                 {
@@ -699,8 +690,8 @@ static stringToComplexError ParseComplexValue(const char* tx, BOOL bConvertByNAN
                     ierr = STRINGTOCOMPLEX_ERROR;
                 }
             }
-        } 
-        
+        }
+
 
         FREE(rnum_string);
         FREE(inum_string);
@@ -948,33 +939,15 @@ static stringToComplexError ParseComplexValueWInPlace(wchar_t* tx, BOOL bConvert
     *real = stringToDoubleWInPlace(tx, FALSE, &ierrDouble);
     *imag = 0;
 
-    /* test on strlen(tx) > 1 to remove case 'e' */
-    if ((int)wcslen(tx) < 2)
+    if (ierrDouble == STRINGTODOUBLE_NO_ERROR)
     {
-        if (ierrDouble == STRINGTODOUBLE_NO_ERROR)
-        {
-            ierr = (stringToComplexError)ierrDouble;
-        }
-        else
-        {
-            if (bConvertByNAN)
-            {
-                ierrDouble = STRINGTODOUBLE_NOT_A_NUMBER;
-                *real = returnNAN();
-                *imag = 0;
-            }
-            else
-            {
-                *real = 0;
-                *imag = 0;
-                ierr = (stringToComplexError)ierrDouble;
-            }
-        }
+        ierr = (stringToComplexError)ierrDouble;
     }
     else if (ierrDouble != STRINGTODOUBLE_NO_ERROR)
     {
         // convert %i to i
-        for (wchar_t *it = tx, *jt = tx; *it != L'\0'; it++, jt++)
+        wchar_t* jt = tx;
+        for (wchar_t* it = tx; *it != L'\0'; it++, jt++)
         {
             if (it[0] == L'%' && it[1] == L'i')
             {
@@ -982,6 +955,7 @@ static stringToComplexError ParseComplexValueWInPlace(wchar_t* tx, BOOL bConvert
             }
             *jt = *it;
         }
+        *jt = '\0';
 
         lnum = ParseNumberW(tx);
         if (lnum <= 1)
@@ -1018,19 +992,38 @@ static stringToComplexError ParseComplexValueWInPlace(wchar_t* tx, BOOL bConvert
                 (inum_string[wcslen(inum_string) - 1] == L'j')) // The imaginary part looks like "a*%i"
         {
             inum_string[wcslen(inum_string) - 1] = L'\0';
-            if (inum_string[wcslen(inum_string) - 1] == L'*')
+            gainImagI = 1.;
+            if (*inum_string == L'\0' && (wcslen(tx) == 0 || tx[wcslen(tx) - 1] == L'-'))
             {
-                inum_string[wcslen(inum_string) - 1] = L'\0';
+                inum_string = L"+1";
+                if (tx[wcslen(tx) - 1] == L'-')
+                {
+                    gainImagI = -1.;
+                }
             }
-
-            // *inum_string will be set to '\0' on leftstringInPlace, store the sign
-            // somewhere before!
-            gainImagI = 1.;
-            if (*inum_string == L'-')
+            else if (inum_string[wcslen(inum_string) - 1] == L'+')
+            {
+                inum_string = L"+1";
+            }
+            else if (inum_string[wcslen(inum_string) - 1] == L'-')
             {
-                gainImagI = -1.;
+                inum_string = L"-1";
+            }
+            else
+            {
+                if (inum_string[wcslen(inum_string) - 1] == L'*')
+                {
+                    inum_string[wcslen(inum_string) - 1] = L'\0';
+                }
+
+                // *inum_string will be set to '\0' on leftstringInPlace, store the sign
+                // somewhere before!
+                if (*inum_string == L'-')
+                {
+                    gainImagI = -1.;
+                }
+                inum_string++;
             }
-            inum_string++;
         }
         else if (inum_string[1] == L'i' || inum_string[1] == L'j') // The imaginary part looks like "%i*a". For instance if string() has been used
         {
@@ -1062,7 +1055,16 @@ static stringToComplexError ParseComplexValueWInPlace(wchar_t* tx, BOOL bConvert
         {
             gainImagI = 0.;
         }
-        rnum_string = leftstringWInPlace(tx, lnum);
+
+        if (wcslen(tx) == 0 || (tx[0] == L'-' && wcslen(tx) == 1))
+        {
+            rnum_string = L"0";
+        }
+        else
+        {
+            rnum_string = leftstringWInPlace(tx, lnum);
+        }
+
 
         if (gainImagI != 0. && *rnum_string != L'\0' && *inum_string == L'\0')
         {