bug 12285: export_to_hdf5 function could not handle any matrix in append mode. 82/10482/4
Antoine ELIAS [Thu, 14 Feb 2013 16:25:51 +0000 (17:25 +0100)]
Change-Id: Ie29862b4e426d8dfa9cf329198c627b45cc69ae1

scilab/CHANGES_5.4.X
scilab/modules/hdf5/includes/h5_writeDataToFile.h
scilab/modules/hdf5/sci_gateway/cpp/sci_export_to_hdf5.cpp
scilab/modules/hdf5/src/c/h5_writeDataToFile.c
scilab/modules/hdf5/tests/nonreg_tests/bug_12285.dia.ref [new file with mode: 0644]
scilab/modules/hdf5/tests/nonreg_tests/bug_12285.tst [new file with mode: 0644]

index 2ec2799..5a3d8f7 100644 (file)
@@ -361,6 +361,8 @@ Bug fixes
 
 * Bug #12284 fixed - Completion in console could to a crash (or deadlock).
 
+* Bug #12285 fixed - export_to_hdf5 function could not handle any matrix in append mode.
+
 * Bug #12304 fixed - Undock, redock and undock led to an exception.
 
 
index c72fce1..7337bd7 100644 (file)
@@ -20,7 +20,7 @@ HDF5_SCILAB_IMPEXP int updateScilabVersion(int _iFile);
 
 HDF5_SCILAB_IMPEXP char* createGroupName(char* _pstGroupName);
 HDF5_SCILAB_IMPEXP char* createPathName(char* _pstGroupName, int _iIndex);
-
+HDF5_SCILAB_IMPEXP int deleteHDF5Var(int _iFile, char* _pstName);
 
 HDF5_SCILAB_IMPEXP int writeDoubleMatrix(int _iFile, char* _pstDatasetName, int _iDims, int* _piDims, double *_pdblData);
 HDF5_SCILAB_IMPEXP int writeDoubleComplexMatrix(int _iFile, char* _pstDatasetName, int _iDims, int* _piDims, double *_pdblReal, double *_pdblImg);
index ba27f34..4ac92b4 100644 (file)
@@ -72,11 +72,12 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
 
     SciErr sciErr;
 
+    int iRhs = nbInputArgument(pvApiCtx);
     CheckInputArgumentAtLeast(pvApiCtx, 1);
-    CheckLhs(0, 1);
+    CheckOutputArgument(pvApiCtx, 0, 1);
 
-    pstNameList = (char**)MALLOC(sizeof(char*) * Rhs);
-    iNbVar = extractVarNameList(1, Rhs, pstNameList);
+    pstNameList = (char**)MALLOC(sizeof(char*) * iRhs);
+    iNbVar = extractVarNameList(1, iRhs, pstNameList);
     if (iNbVar == 0)
     {
         FREE(pstNameList);
@@ -84,7 +85,7 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
     }
 
     piAddrList = (int**)MALLOC(sizeof(int*) * (iNbVar));
-    for (int i = 1 ; i < Rhs ; i++)
+    for (int i = 1 ; i < iRhs ; i++)
     {
         if (strcmp(pstNameList[i], "-append") == 0)
         {
@@ -159,28 +160,43 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
             //import all data
             for (int i = 0 ; i < iNbItem ; i++)
             {
-                for (int j = 1 ; j < Rhs ; j++)
+                for (int j = 1 ; j < iRhs ; j++)
                 {
-                    if (strcmp(pstNameList[i], "-append") == 0)
+                    if (strcmp(pstNameList[j], "-append") == 0)
                     {
                         continue;
                     }
 
                     if (strcmp(pstVarNameList[i], pstNameList[j]) == 0)
                     {
-                        closeHDF5File(iH5File);
-                        Scierror(999, _("%s: Variable \'%s\' already exists in file \'%s\'."), fname, pstVarNameList[i], pstNameList[0]);
-                        return 1;
+                        if (bAppendMode == false)
+                        {
+                            closeHDF5File(iH5File);
+                            Scierror(999, _("%s: Variable \'%s\' already exists in file \'%s\'\nUse -append option to replace existing variable\n."), fname, pstVarNameList[i], pstNameList[0]);
+                            return 1;
+                        }
+                        else
+                        {
+                            if (deleteHDF5Var(iH5File, pstVarNameList[i]))
+                            {
+                                //failed to clean
+                                Scierror(999, _("%s: Unable to delete existing variable \"%s\"."), fname, pstVarNameList[i]);
+                                return 1;
+                            }
+                            continue;
+                        }
                     }
                 }
+
                 FREE(pstVarNameList[i]);
             }
+
             FREE(pstVarNameList);
         }
     }
 
     // export data
-    for (int i = 1 ; i < Rhs ; i++)
+    for (int i = 1 ; i < iRhs ; i++)
     {
         if (strcmp(pstNameList[i], "-append") == 0)
         {
@@ -194,7 +210,7 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
         }
     }
 
-    if (bExport && Rhs != 1)
+    if (bExport && iRhs != 1)
     {
         //add or update scilab version and file version in hdf5 file
         if (updateScilabVersion(iH5File) < 0)
@@ -214,7 +230,9 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
 
     //close hdf5 file
     closeHDF5File(iH5File);
-    if (bExport == false && Rhs != 1)
+
+    //delete file in case of error but nor in append mode
+    if (bExport == false && bAppendMode == false && iRhs != 1)
     {
         //remove file
         deleteafile(pstFileName);
@@ -223,14 +241,14 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
 
     //create boolean return value
     int *piReturn = NULL;
-    sciErr = allocMatrixOfBoolean(pvApiCtx, Rhs + 1, 1, 1, &piReturn);
+    sciErr = allocMatrixOfBoolean(pvApiCtx, iRhs + 1, 1, 1, &piReturn);
     if (sciErr.iErr)
     {
         printError(&sciErr, 0);
         return 1;
     }
 
-    if (bExport == true || Rhs == 1)
+    if (bExport == true || iRhs == 1)
     {
         piReturn[0] = 1;
     }
@@ -241,7 +259,7 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
 
 
     //free memory
-    for (int i = 0 ; i < Rhs ; i++)
+    for (int i = 0 ; i < iRhs ; i++)
     {
         FREE(pstNameList[i]);
     }
@@ -249,7 +267,7 @@ int sci_export_to_hdf5(char *fname, unsigned long fname_len)
 
     FREE(piAddrList);
 
-    LhsVar(1) = Rhs + 1;
+    LhsVar(1) = iRhs + 1;
     PutLhsVar();
     return 0;
 }
index d592397..f9a1c4e 100644 (file)
@@ -1544,7 +1544,7 @@ int writeBooleanSparseMatrix(int _iFile, char *_pstDatasetName, int _iRows, int
     }
 
     pstColPath = createPathName(pstGroupName, 1);
-    if(_iNbItem != 0)
+    if (_iNbItem != 0)
     {
         status = writeInteger32Matrix(_iFile, pstColPath, 1, &_iNbItem, _piColPos);
         if (status < 0)
@@ -1653,7 +1653,7 @@ void *openList(int _iFile, char *pstDatasetName, int _iNbItem)
 
     if (_iNbItem)
     {
-        pobjArray = MALLOC(sizeof(hobj_ref_t) * _iNbItem);
+        pobjArray = (hobj_ref_t*)MALLOC(sizeof(hobj_ref_t) * _iNbItem);
     }
 
     return pobjArray;
@@ -1787,3 +1787,98 @@ int closeList(int _iFile, void *_pvList, char *_pstListName, int _iNbItem, int _
     FREE(_pvList);
     return 0;
 }
+
+static int deleteHDF5group(int _iFile, char* _pstName)
+{
+    hid_t status = 0;
+    //open group
+    char* pstGroupName = createGroupName(_pstName);
+    hid_t groupID = H5Gopen(_iFile, pstGroupName);
+    if (groupID >= 0)
+    {
+        int i = 0;
+        H5G_info_t groupInfo;
+        //get children count
+        status = H5Gget_info(groupID, &groupInfo);
+        if (status < 0)
+        {
+            return -1;
+        }
+
+        //for each child,
+        for (i = 0 ; i < groupInfo.nlinks ; i++)
+        {
+            ssize_t size = 0;
+            char* pstPathName = NULL;
+            char* pstChildName = NULL;
+            //build child path
+            pstPathName = createPathName(pstGroupName, i);
+
+            //try to delete child and his children
+            deleteHDF5group(_iFile, pstPathName);
+
+            //get child name
+            size = H5Lget_name_by_idx(groupID, ".", H5_INDEX_NAME, H5_ITER_INC, 0, 0, 0, H5P_DEFAULT) + 1;
+            pstChildName = (char*)MALLOC(sizeof(char) * size);
+            H5Lget_name_by_idx(groupID, ".", H5_INDEX_NAME, H5_ITER_INC, 0, pstChildName, size, H5P_DEFAULT);
+
+            //unlink child
+            status = H5Ldelete(groupID, pstChildName, H5P_DEFAULT);
+
+            FREE(pstChildName);
+            FREE(pstPathName);
+
+            if (status < 0)
+            {
+                return 1;
+            }
+        }
+
+        //close group
+        status = H5Gclose(groupID);
+        if (status < 0)
+        {
+            return -1;
+        }
+
+        //delete group
+        status = H5Ldelete(_iFile, pstGroupName, H5P_DEFAULT);
+        if (status < -1)
+        {
+            return -1;
+        }
+
+    }
+
+    FREE(pstGroupName);
+    return 0;
+}
+
+//According to 5.5.2. Deleting a Dataset from a File and Reclaiming Space of http://www.hdfgroup.org/HDF5/doc/UG/10_Datasets.html
+//it is actually impossible to really remove data from HDF5 file so unlink dataset to main group
+int deleteHDF5Var(int _iFile, char* _pstName)
+{
+    herr_t status = 0;
+    void *oldclientdata = NULL;
+    H5E_auto2_t oldfunc;
+
+    /* Save old error handler */
+    H5Eget_auto2(H5E_DEFAULT, &oldfunc, &oldclientdata);
+
+    /* Turn off error handling */
+    H5Eset_auto2(H5E_DEFAULT, NULL, NULL);
+
+    //try to unlink potential subgroups
+    deleteHDF5group(_iFile, _pstName);
+
+    //delete current dataset link
+    status = H5Ldelete(_iFile, _pstName, H5P_DEFAULT);
+    if (status < 0)
+    {
+        H5Eset_auto2(H5E_DEFAULT, oldfunc, oldclientdata);
+        return status;
+    }
+
+    H5Eset_auto2(H5E_DEFAULT, oldfunc, oldclientdata);
+    return 0;
+}
diff --git a/scilab/modules/hdf5/tests/nonreg_tests/bug_12285.dia.ref b/scilab/modules/hdf5/tests/nonreg_tests/bug_12285.dia.ref
new file mode 100644 (file)
index 0000000..0c50054
--- /dev/null
@@ -0,0 +1,53 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2011 - DIGITEO - Antoine ELIAS
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- Non-regression test for bug 12285 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/show_bug.cgi?id=12285
+//
+// <-- Short Description -->
+// 
+// export_to_hdf5 function could not handle any matrix in append mode.
+savefile = TMPDIR + "bug_12285.sod";
+a_list = list(1,2,3);
+a_list_ref = a_list;
+a_string = "a string";
+a_string_ref = a_string;
+an_integer = 546;
+an_integer_ref = an_integer;
+a_complex_num = 1 + %i;
+a_complex_num_ref = a_complex_num;
+an_array_multidim  = [1 2 3 4; 5 6 7 8; 9 10 11 12];
+an_array_multidim_ref = an_array_multidim;
+//save all variables in the same file
+assert_checktrue(export_to_hdf5(savefile, 'a_list', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'a_string', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'an_integer', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'a_complex_num', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'an_array_multidim', '-append'));
+//clear variables
+clear a_list a_string an_integer an_array_multidim
+//load variables
+assert_checktrue(import_from_hdf5(savefile, 'a_list'));
+assert_checktrue(import_from_hdf5(savefile, 'a_string'));
+assert_checktrue(import_from_hdf5(savefile, 'an_integer'));
+assert_checktrue(import_from_hdf5(savefile, 'a_complex_num'));
+assert_checktrue(import_from_hdf5(savefile, 'an_array_multidim'));
+//check variables values
+assert_checkequal(a_list, a_list_ref);
+assert_checkequal(a_string, a_string_ref);
+assert_checkequal(an_integer, an_integer_ref);
+assert_checkequal(a_complex_num, a_complex_num_ref);
+assert_checkequal(an_array_multidim, an_array_multidim_ref);
+//append file with a already existing variable name but different data
+a_list = "ok ok i am not a list";
+a_list_ref = a_list;
+assert_checktrue(export_to_hdf5(savefile, 'a_list', '-append'));
+clear a_list
+assert_checktrue(import_from_hdf5(savefile, 'a_list'));
+assert_checkequal(a_list, a_list_ref);
diff --git a/scilab/modules/hdf5/tests/nonreg_tests/bug_12285.tst b/scilab/modules/hdf5/tests/nonreg_tests/bug_12285.tst
new file mode 100644 (file)
index 0000000..4b40c5f
--- /dev/null
@@ -0,0 +1,63 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2011 - DIGITEO - Antoine ELIAS
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- Non-regression test for bug 12285 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/show_bug.cgi?id=12285
+//
+// <-- Short Description -->
+// 
+// export_to_hdf5 function could not handle any matrix in append mode.
+
+savefile = TMPDIR + "bug_12285.sod";
+a_list = list(1,2,3);
+a_list_ref = a_list;
+
+a_string = "a string";
+a_string_ref = a_string;
+
+an_integer = 546;
+an_integer_ref = an_integer;
+
+a_complex_num = 1 + %i;
+a_complex_num_ref = a_complex_num;
+
+an_array_multidim  = [1 2 3 4; 5 6 7 8; 9 10 11 12];
+an_array_multidim_ref = an_array_multidim;
+
+//save all variables in the same file
+assert_checktrue(export_to_hdf5(savefile, 'a_list', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'a_string', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'an_integer', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'a_complex_num', '-append'));
+assert_checktrue(export_to_hdf5(savefile, 'an_array_multidim', '-append'));
+
+//clear variables
+clear a_list a_string an_integer an_array_multidim
+
+//load variables
+assert_checktrue(import_from_hdf5(savefile, 'a_list'));
+assert_checktrue(import_from_hdf5(savefile, 'a_string'));
+assert_checktrue(import_from_hdf5(savefile, 'an_integer'));
+assert_checktrue(import_from_hdf5(savefile, 'a_complex_num'));
+assert_checktrue(import_from_hdf5(savefile, 'an_array_multidim'));
+
+//check variables values
+assert_checkequal(a_list, a_list_ref);
+assert_checkequal(a_string, a_string_ref);
+assert_checkequal(an_integer, an_integer_ref);
+assert_checkequal(a_complex_num, a_complex_num_ref);
+assert_checkequal(an_array_multidim, an_array_multidim_ref);
+
+//append file with a already existing variable name but different data
+a_list = "ok ok i am not a list";
+a_list_ref = a_list;
+assert_checktrue(export_to_hdf5(savefile, 'a_list', '-append'));
+clear a_list
+assert_checktrue(import_from_hdf5(savefile, 'a_list'));
+assert_checkequal(a_list, a_list_ref);