Hello,
On 2015-03-08 22:52, Elena Pourmal wrote:
Your patch was reviewed before 1.8.14 release but didn’t make it into the
1.8.14 release due to a few missing items (see below).
I've attached a new version of the patch.
Would it be possible for you to create:
1. A test for this new functionality that passes at least on Linux and
Windows.
Please take a look at the test/dset.c and test/external.c files and add
the new tests where they fit best. Separate test file is also OK, but then you
will need to modify
autoconf and CMake related files, and I don’t think it makes sense in
this particular case.
I've added tests in test/external.c which test absolute and relative
filenames and on windows path-absolute (\dir\path).
The tests work on Linux and Windows.
2. Add the tests to the h5dump, h5ls, h5diff and h5repack tools.
I'm not sure what test to add here, as the functionality of these tools
is not affected. (The output of h5dump / h5ls / h5diff should not change
in any way, and h5repack should not do anything different.)
3. A short paragraph that describes the issue and the fix for it for the
RELEASE.txt file.
I've added that.
4. Enhancements to the HDF5 Reference Manual and User’s Guide entries for
H5Pset_external where applicable.
(I'm not sure where the sources for the Reference Manual and the User's
Guide are, so I'm not sure who will apply these changes / how I can
submit patches for that.)
http://www.hdfgroup.org/HDF5/doc/RM/RM_H5P.html#Property-SetExternal
A paragraph can be added to the Description:
If 'name' is a relative filename, it will be considered relative to the
HDF5 file. In this case when the HDF5 file is moved, the external files
should be moved along with it.
And to History:
1.8.15 Relative filenames are now considered relative to the HDF5
file, not to the current directory.
http://www.hdfgroup.org/HDF5/doc/UG/10_Datasets.html
To last paragraph of "5.5.4. External Storage Properties":
All referenced external data files must exist before performing raw data
I/O on the dataset. This is normally not a problem since those files are
being managed directly by the application or indirectly through some
other library. However, if the file is transferred from its original
context, care must be taken to assure that all the external files are
accessible in the new location.
Add:
If relative filenames are used, they will be considered relative to the
location of the HDF5 file.
Code freeze for 1.8.15 is April 1, 2015. We will need the patches for 1. and 2.
before March 25 to tests it on the platforms beyond Linux and Windows.
Documentation patches should be provided by April 15.
Best regards,
Steffen Kieß
Index: release_docs/RELEASE.txt
===================================================================
--- release_docs/RELEASE.txt (revision 26434)
+++ release_docs/RELEASE.txt (working copy)
@@ -692,6 +692,8 @@
(PVN - 2009/01/8)
- Added code to maintain a min_clean_fraction in the metadata cache when
in serial mode. (MAM - 2009/01/9)
+ - Interpret filesnames for external files as relative to the HDF5 file,
+ not relative to the current directory. HDFFV-8740
Index: src/H5Defl.c
===================================================================
--- src/H5Defl.c (revision 26434)
+++ src/H5Defl.c (working copy)
@@ -33,6 +33,7 @@
#include "H5Eprivate.h" /* Error handling */
#include "H5Fprivate.h" /* Files */
#include "H5HLprivate.h" /* Local Heaps */
+#include "H5MMprivate.h" /* Memory management */
#include "H5VMprivate.h" /* Vector and array functions */
@@ -48,6 +49,7 @@
/* Callback info for readvv operation */
typedef struct H5D_efl_readvv_ud_t {
const H5O_efl_t *efl; /* Pointer to efl info */
+ H5F_t *file; /* File the data set is in */
unsigned char *rbuf; /* Read buffer */
} H5D_efl_readvv_ud_t;
@@ -54,6 +56,7 @@
/* Callback info for writevv operation */
typedef struct H5D_efl_writevv_ud_t {
const H5O_efl_t *efl; /* Pointer to efl info */
+ H5F_t *file; /* File the data set is in */
const unsigned char *wbuf; /* Write buffer */
} H5D_efl_writevv_ud_t;
@@ -75,9 +78,9 @@
size_t mem_max_nseq, size_t *mem_curr_seq, size_t mem_len_arr[], hsize_t mem_offset_arr[]);
/* Helper routines */
-static herr_t H5D__efl_read(const H5O_efl_t *efl, haddr_t addr, size_t size,
+static herr_t H5D__efl_read(const H5O_efl_t *efl, H5F_t *file, haddr_t addr, size_t size,
uint8_t *buf);
-static herr_t H5D__efl_write(const H5O_efl_t *efl, haddr_t addr, size_t size,
+static herr_t H5D__efl_write(const H5O_efl_t *efl, H5F_t *file, haddr_t addr, size_t size,
const uint8_t *buf);
@@ -110,6 +113,58 @@
+/*--------------------------------------------------------------------------
+ * Function: H5D_build_name
+ *
+ * Purpose: Prepend extpath to external file name (for relative files) and store in FULL_NAME
+ *
+ * Return: Non-negative on success/Negative on failure
+ *
+ * Programmer: Steffen Kiess
+ * March 10, 2015
+--------------------------------------------------------------------------*/
+static herr_t
+H5D_build_name(const H5F_t *file, const char* filename, char **full_name/*out*/)
+{
+ char *extpath; /* absolute path of directory the HDF5 file is in */
+ size_t extpath_len; /* length of extpath */
+ size_t filename_len; /* length of external file name */
+ herr_t ret_value = SUCCEED; /* Return value */
+
+ FUNC_ENTER_NOAPI_NOINIT
+
+ HDassert(file);
+ extpath = H5F_EXTPATH(file);
+ HDassert(extpath);
+ extpath_len = HDstrlen(extpath);
+ filename_len = HDstrlen(filename);
+
+ if (H5_CHECK_ABSOLUTE(filename)) {
+ /* If the filename is absolute, simply use filename */
+ if(NULL == (*full_name = (char *)H5MM_strdup(filename)))
+ HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed")
+ } else if (H5_CHECK_ABS_PATH(filename)) {
+ /* On windows: For path absolute names: Use the drive letter of extpath + the filename */
+ if(NULL == (*full_name = (char *)H5MM_malloc(filename_len + 3)))
+ HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate filename buffer")
+ HDsnprintf(*full_name, (filename_len + 3), "%c:%s", extpath[0], filename);
+
+ /* No support for filenames like "d:path\file" */
+ } else {
+ /* Relative filename: Allocate a buffer to hold the filename + extpath + possibly the delimiter + terminating null byte */
+ if(NULL == (*full_name = (char *)H5MM_malloc(extpath_len + filename_len + 2)))
+ HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate filename buffer")
+
+ /* Compose the full file name */
+ HDsnprintf(*full_name, (extpath_len + filename_len + 2), "%s%s%s", extpath,
+ (H5_CHECK_DELIMITER(extpath[extpath_len - 1]) ? "" : H5_DIR_SEPS), filename);
+ }
+
+done:
+ FUNC_LEAVE_NOAPI(ret_value)
+} /* H5D_build_name() */
+
+
/*-------------------------------------------------------------------------
* Function: H5D__efl_construct
*
@@ -254,7 +309,7 @@
*-------------------------------------------------------------------------
*/
static herr_t
-H5D__efl_read(const H5O_efl_t *efl, haddr_t addr, size_t size, uint8_t *buf)
+H5D__efl_read(const H5O_efl_t *efl, H5F_t *file, haddr_t addr, size_t size, uint8_t *buf)
{
int fd = -1;
size_t to_read;
@@ -266,6 +321,7 @@
ssize_t n;
size_t u; /* Local index variable */
herr_t ret_value = SUCCEED; /* Return value */
+ char *full_name = NULL; /* File name with prefix */
FUNC_ENTER_STATIC
@@ -291,7 +347,9 @@
HGOTO_ERROR(H5E_EFL, H5E_OVERFLOW, FAIL, "read past logical end of file")
if(H5F_OVERFLOW_HSIZET2OFFT(efl->slot[u].offset + skip))
HGOTO_ERROR(H5E_EFL, H5E_OVERFLOW, FAIL, "external file address overflowed")
- if((fd = HDopen(efl->slot[u].name, O_RDONLY, 0)) < 0)
+ if(H5D_build_name(file, efl->slot[u].name, &full_name/*out*/) < 0)
+ HGOTO_ERROR(H5E_EFL, H5E_CANTGET, FAIL, "can't build external file name")
+ if((fd = HDopen(full_name, O_RDONLY, 0)) < 0)
HGOTO_ERROR(H5E_EFL, H5E_CANTOPENFILE, FAIL, "unable to open external raw data file")
if(HDlseek(fd, (off_t)(efl->slot[u].offset + skip), SEEK_SET) < 0)
HGOTO_ERROR(H5E_EFL, H5E_SEEKERROR, FAIL, "unable to seek in external raw data file")
@@ -315,6 +373,7 @@
} /* end while */
done:
+ full_name = (char *)H5MM_xfree(full_name);
if(fd >= 0)
HDclose(fd);
@@ -341,7 +400,7 @@
*-------------------------------------------------------------------------
*/
static herr_t
-H5D__efl_write(const H5O_efl_t *efl, haddr_t addr, size_t size, const uint8_t *buf)
+H5D__efl_write(const H5O_efl_t *efl, H5F_t *file, haddr_t addr, size_t size, const uint8_t *buf)
{
int fd = -1;
size_t to_write;
@@ -352,6 +411,7 @@
hsize_t skip = 0;
size_t u; /* Local index variable */
herr_t ret_value = SUCCEED; /* Return value */
+ char *full_name = NULL; /* File name with prefix */
FUNC_ENTER_STATIC
@@ -377,8 +437,10 @@
HGOTO_ERROR(H5E_EFL, H5E_OVERFLOW, FAIL, "write past logical end of file")
if(H5F_OVERFLOW_HSIZET2OFFT(efl->slot[u].offset + skip))
HGOTO_ERROR(H5E_EFL, H5E_OVERFLOW, FAIL, "external file address overflowed")
- if((fd = HDopen(efl->slot[u].name, O_CREAT | O_RDWR, 0666)) < 0) {
- if(HDaccess(efl->slot[u].name, F_OK) < 0)
+ if(H5D_build_name(file, efl->slot[u].name, &full_name/*out*/) < 0)
+ HGOTO_ERROR(H5E_EFL, H5E_CANTGET, FAIL, "can't build external file name")
+ if((fd = HDopen(full_name, O_CREAT | O_RDWR, 0666)) < 0) {
+ if(HDaccess(full_name, F_OK) < 0)
HGOTO_ERROR(H5E_EFL, H5E_CANTOPENFILE, FAIL, "external raw data file does not exist")
else
HGOTO_ERROR(H5E_EFL, H5E_CANTOPENFILE, FAIL, "unable to open external raw data file")
@@ -403,6 +465,7 @@
} /* end while */
done:
+ full_name = (char *)H5MM_xfree(full_name);
if(fd >= 0)
HDclose(fd);
@@ -431,7 +494,7 @@
FUNC_ENTER_STATIC
/* Read data */
- if(H5D__efl_read(udata->efl, dst_off, len, (udata->rbuf + src_off)) < 0)
+ if(H5D__efl_read(udata->efl, udata->file, dst_off, len, (udata->rbuf + src_off)) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "EFL read failed")
done:
@@ -468,6 +531,8 @@
HDassert(io_info);
HDassert(io_info->store->efl.nused > 0);
HDassert(io_info->u.rbuf);
+ HDassert(io_info->dset);
+ HDassert(io_info->dset->oloc.file);
HDassert(dset_curr_seq);
HDassert(dset_len_arr);
HDassert(dset_off_arr);
@@ -477,6 +542,7 @@
/* Set up user data for H5VM_opvv() */
udata.efl = &(io_info->store->efl);
+ udata.file = io_info->dset->oloc.file;
udata.rbuf = (unsigned char *)io_info->u.rbuf;
/* Call generic sequence operation routine */
@@ -511,7 +577,7 @@
FUNC_ENTER_STATIC
/* Write data */
- if(H5D__efl_write(udata->efl, dst_off, len, (udata->wbuf + src_off)) < 0)
+ if(H5D__efl_write(udata->efl, udata->file, dst_off, len, (udata->wbuf + src_off)) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "EFL write failed")
done:
@@ -548,6 +614,8 @@
HDassert(io_info);
HDassert(io_info->store->efl.nused > 0);
HDassert(io_info->u.wbuf);
+ HDassert(io_info->dset);
+ HDassert(io_info->dset->oloc.file);
HDassert(dset_curr_seq);
HDassert(dset_len_arr);
HDassert(dset_off_arr);
@@ -557,6 +625,7 @@
/* Set up user data for H5VM_opvv() */
udata.efl = &(io_info->store->efl);
+ udata.file = io_info->dset->oloc.file;
udata.wbuf = (const unsigned char *)io_info->u.wbuf;
/* Call generic sequence operation routine */
Index: src/H5Fquery.c
===================================================================
--- src/H5Fquery.c (revision 26434)
+++ src/H5Fquery.c (working copy)
@@ -158,7 +158,7 @@
* Function: H5F_get_extpath
*
* Purpose: Retrieve the file's 'extpath' flags
- * This is used by H5L_extern_traverse() to retrieve the main file's location
+ * This is used by H5L_extern_traverse() and H5D_build_name() to retrieve the main file's location
* when searching the target file.
*
* Return: 'extpath' on success/abort on failure (shouldn't fail)
Index: src/H5system.c
===================================================================
--- src/H5system.c (revision 26434)
+++ src/H5system.c (working copy)
@@ -729,7 +729,7 @@
* Function: H5_build_extpath
*
* Purpose: To build the path for later searching of target file for external
- * link. This path can be either:
+ * links and external files. This path can be either:
* 1. The absolute path of NAME
* or
* 2. The current working directory + relative path of NAME
Index: test/external.c
===================================================================
--- test/external.c (revision 26434)
+++ test/external.c (working copy)
@@ -30,6 +30,8 @@
"extern_2",
"extern_3",
"extern_4",
+ "extern_5",
+ "extern_dir/file_1",
NULL
};
@@ -919,6 +921,173 @@
/*-------------------------------------------------------------------------
+ * Function: test_5
+ *
+ * Purpose: Test absolute filenames for external files.
+ * This will create an HDF5 file in a subdirectory which will
+ * refer to /full/path/extern_*a.raw on unix and to
+ * c:\full\path\extern_*a.raw and \full\path\extern_*a.raw on
+ * windows.
+ *
+ * Return: Success: 0
+ *
+ * Failure: number of errors
+ *
+ * Programmer: Steffen Kiess
+ * March 10, 2015
+ *
+ * Modifications:
+ *
+ *-------------------------------------------------------------------------
+ */
+static int
+test_5 (hid_t fapl)
+{
+ hid_t file=-1; /*file to write to */
+ hid_t dcpl=-1; /*dataset creation properties */
+ hid_t space=-1; /*data space */
+ hid_t dset=-1; /*dataset */
+ size_t i; /*miscellaneous counters */
+ char cwdpath[1024]; /*working directory */
+ char filename[1024]; /*file names */
+ int part[25], whole[100]; /*raw data buffers */
+ hsize_t cur_size; /*current data space size */
+
+ TESTING("absolute filenames for external file");
+
+ h5_fixname(FILENAME[4], fapl, filename, sizeof filename);
+ if ((file=H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) FAIL_STACK_ERROR;
+
+ /* Create the dataset */
+ if((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) goto error;
+ if(NULL == HDgetcwd(cwdpath, sizeof (cwdpath)))
+ TEST_ERROR
+ for (i = 0; i < 4; i++) {
+ HDsnprintf(filename, sizeof(filename), "%s%sextern_%da.raw", cwdpath, H5_DIR_SEPS, (int) i + 1);
+#if defined(H5_HAVE_WINDOW_PATH)
+ // For windows, test path-absolute case (\dir\file.raw) for the second file
+ if (i == 1)
+ HDsnprintf(filename, sizeof(filename), "%s%sextern_%da.raw", cwdpath + 2, H5_DIR_SEPS, i + 1);
+#endif
+ if(H5Pset_external(dcpl, filename, (off_t)(i * 10), (hsize_t)sizeof part) < 0)
+ goto error;
+ }
+ cur_size = 100;
+ if((space = H5Screate_simple(1, &cur_size, NULL)) < 0) goto error;
+ if((dset = H5Dcreate2(file, "dset1", H5T_NATIVE_INT, space, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) goto error;
+
+ /*
+ * Read the entire dataset and compare with the original
+ */
+ memset(whole, 0, sizeof(whole));
+ if(H5Dread(dset, H5T_NATIVE_INT, space, space, H5P_DEFAULT, whole) < 0) goto error;
+ for(i = 0; i < 100; i++)
+ if(whole[i] != (signed)i) {
+ H5_FAILED();
+ puts(" Incorrect value(s) read.");
+ goto error;
+ } /* end if */
+
+ if (H5Dclose(dset) < 0) goto error;
+ if (H5Pclose(dcpl) < 0) goto error;
+ if (H5Sclose(space) < 0) goto error;
+ if (H5Fclose(file) < 0) goto error;
+ PASSED();
+ return 0;
+
+ error:
+ H5E_BEGIN_TRY {
+ H5Dclose(dset);
+ H5Pclose(dcpl);
+ H5Sclose(space);
+ H5Fclose(file);
+ } H5E_END_TRY;
+ return 1;
+}
+
+
+/*-------------------------------------------------------------------------
+ * Function: test_6
+ *
+ * Purpose: Test relative filenames for external files.
+ * This will create an HDF5 file in a subdirectory which will
+ * refer to ../extern_*a.raw
+ *
+ * Return: Success: 0
+ *
+ * Failure: number of errors
+ *
+ * Programmer: Steffen Kiess
+ * March 10, 2015
+ *
+ * Modifications:
+ *
+ *-------------------------------------------------------------------------
+ */
+static int
+test_6 (hid_t fapl)
+{
+ hid_t file=-1; /*file to write to */
+ hid_t dcpl=-1; /*dataset creation properties */
+ hid_t space=-1; /*data space */
+ hid_t dset=-1; /*dataset */
+ size_t i; /*miscellaneous counters */
+ char cwdpath[1024]; /*working directory */
+ char filename[1024]; /*file names */
+ int part[25], whole[100]; /*raw data buffers */
+ hsize_t cur_size; /*current data space size */
+
+ TESTING("relative filenames for external file");
+
+ if (HDmkdir("extern_dir", (mode_t)0755) < 0 && errno != EEXIST) TEST_ERROR;
+
+ h5_fixname(FILENAME[5], fapl, filename, sizeof filename);
+ if ((file=H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) FAIL_STACK_ERROR;
+
+ /* Create the dataset */
+ if((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) goto error;
+ if(NULL == HDgetcwd(cwdpath, sizeof (cwdpath)))
+ TEST_ERROR
+ for (i = 0; i < 4; i++) {
+ HDsnprintf(filename, sizeof(filename), "..%sextern_%da.raw", H5_DIR_SEPS, (int) i + 1);
+ if(H5Pset_external(dcpl, filename, (off_t)(i * 10), (hsize_t)sizeof part) < 0)
+ goto error;
+ }
+ cur_size = 100;
+ if((space = H5Screate_simple(1, &cur_size, NULL)) < 0) goto error;
+ if((dset = H5Dcreate2(file, "dset1", H5T_NATIVE_INT, space, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) goto error;
+
+ /*
+ * Read the entire dataset and compare with the original
+ */
+ memset(whole, 0, sizeof(whole));
+ if(H5Dread(dset, H5T_NATIVE_INT, space, space, H5P_DEFAULT, whole) < 0) goto error;
+ for(i = 0; i < 100; i++)
+ if(whole[i] != (signed)i) {
+ H5_FAILED();
+ puts(" Incorrect value(s) read.");
+ goto error;
+ } /* end if */
+
+ if (H5Dclose(dset) < 0) goto error;
+ if (H5Pclose(dcpl) < 0) goto error;
+ if (H5Sclose(space) < 0) goto error;
+ if (H5Fclose(file) < 0) goto error;
+ PASSED();
+ return 0;
+
+ error:
+ H5E_BEGIN_TRY {
+ H5Dclose(dset);
+ H5Pclose(dcpl);
+ H5Sclose(space);
+ H5Fclose(file);
+ } H5E_END_TRY;
+ return 1;
+}
+
+
+/*-------------------------------------------------------------------------
* Function: main
*
* Purpose: Runs external dataset tests.
@@ -961,6 +1130,8 @@
nerrors += test_2(fapl);
nerrors += test_3(fapl);
nerrors += test_4(fapl);
+ nerrors += test_5(fapl);
+ nerrors += test_6(fapl);
/* Verify symbol table messages are cached */
nerrors += (h5_verify_cached_stabs(FILENAME, fapl) < 0 ? 1 : 0);
@@ -978,6 +1149,7 @@
remove("extern_3b.raw");
remove("extern_4a.raw");
remove("extern_4b.raw");
+ HDrmdir("extern_dir");
}
return 0;
_______________________________________________
Hdf-forum is for HDF software users discussion.
[email protected]
http://lists.hdfgroup.org/mailman/listinfo/hdf-forum_lists.hdfgroup.org
Twitter: https://twitter.com/hdf5
