summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVailin Choi <vchoi@jam.ad.hdfgroup.org>2017-08-23 21:23:09 (GMT)
committerVailin Choi <vchoi@jam.ad.hdfgroup.org>2017-08-30 15:27:03 (GMT)
commitb9f294a9440d72ce55cd85340583a56b10c1de25 (patch)
treeaa642f2271bdf42df8ded355a625304e171839f4
parentf6c94f3ba60d071ce1ba8b731a66e47fbc39c095 (diff)
downloadhdf5-b9f294a9440d72ce55cd85340583a56b10c1de25.zip
hdf5-b9f294a9440d72ce55cd85340583a56b10c1de25.tar.gz
hdf5-b9f294a9440d72ce55cd85340583a56b10c1de25.tar.bz2
Modifications based on comments from pull request review
(1) Remove unnecessary asserts (2) Add code to insert bad offset values to the test file in gen_bad_offset.c
-rw-r--r--src/H5Glink.c21
-rw-r--r--src/H5Gnode.c43
-rw-r--r--src/H5Gstab.c6
-rw-r--r--src/H5Oefl.c3
-rw-r--r--test/bad_offset.h5bin3312 -> 3312 bytes
-rw-r--r--test/gen_bad_offset.c41
-rw-r--r--test/tmisc.c2
7 files changed, 65 insertions, 51 deletions
diff --git a/src/H5Glink.c b/src/H5Glink.c
index 77b69cb..e199f89 100644
--- a/src/H5Glink.c
+++ b/src/H5Glink.c
@@ -224,6 +224,7 @@ herr_t
H5G__ent_to_link(H5O_link_t *lnk, const H5HL_t *heap,
const H5G_entry_t *ent, const char *name)
{
+ hbool_t dup_soft = FALSE; /* xstrdup the symbolic link name or not */
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_PACKAGE
@@ -238,19 +239,21 @@ H5G__ent_to_link(H5O_link_t *lnk, const H5HL_t *heap,
lnk->cset = H5F_DEFAULT_CSET;
lnk->corder = 0;
lnk->corder_valid = FALSE; /* Creation order not valid for this link */
- lnk->name = H5MM_xstrdup(name);
- HDassert(lnk->name);
+ if((lnk->name = H5MM_xstrdup(name)) == NULL)
+ HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "unable to duplicate link name")
/* Object is a symbolic or hard link */
if(ent->type == H5G_CACHED_SLINK) {
const char *s; /* Pointer to link value */
if((s = (const char *)H5HL_offset_into(heap, ent->cache.slink.lval_offset)) == NULL)
- HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "unable to get link name")
- HDassert(s);
+ HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "unable to get symbolic link name")
/* Copy the link value */
- lnk->u.soft.name = H5MM_xstrdup(s);
+ if((lnk->u.soft.name = H5MM_xstrdup(s)) == NULL)
+ HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "unable to duplicate symbolic link name")
+
+ dup_soft = TRUE;
/* Set link type */
lnk->type = H5L_TYPE_SOFT;
@@ -264,8 +267,12 @@ H5G__ent_to_link(H5O_link_t *lnk, const H5HL_t *heap,
} /* end else */
done:
- if(ret_value < 0 && lnk->name)
- H5MM_xfree(lnk->name);
+ if(ret_value < 0) {
+ if(lnk->name)
+ H5MM_xfree(lnk->name);
+ if(ent->type == H5G_CACHED_SLINK && dup_soft)
+ H5MM_xfree(lnk->u.soft.name);
+ }
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5G__ent_to_link() */
diff --git a/src/H5Gnode.c b/src/H5Gnode.c
index 4ddcf53..76e2c4b 100644
--- a/src/H5Gnode.c
+++ b/src/H5Gnode.c
@@ -35,7 +35,7 @@
/***********/
/* Headers */
/***********/
-#include "H5private.h" /* Generic Functionsi */
+#include "H5private.h" /* Generic Functions */
#include "H5ACprivate.h" /* Metadata cache */
#include "H5Eprivate.h" /* Error handling */
#include "H5Fprivate.h" /* File access */
@@ -396,7 +396,6 @@ H5G_node_cmp2(void *_lt_key, void *_udata, void *_rt_key)
H5G_node_key_t *lt_key = (H5G_node_key_t *) _lt_key;
H5G_node_key_t *rt_key = (H5G_node_key_t *) _rt_key;
const char *s1, *s2;
- const char *base; /* Base of heap */
int ret_value = SUCCEED; /* Return value */
FUNC_ENTER_NOAPI_NOINIT
@@ -406,10 +405,6 @@ H5G_node_cmp2(void *_lt_key, void *_udata, void *_rt_key)
HDassert(lt_key);
HDassert(rt_key);
- /* Get base address of heap */
- base = (const char *)H5HL_offset_into(udata->heap, (size_t)0);
- HDassert(base);
-
/* Get pointers to string names */
if((s1 = (const char *)H5HL_offset_into(udata->heap, lt_key->offset)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get key name")
@@ -458,7 +453,6 @@ H5G_node_cmp3(void *_lt_key, void *_udata, void *_rt_key)
H5G_node_key_t *lt_key = (H5G_node_key_t *) _lt_key;
H5G_node_key_t *rt_key = (H5G_node_key_t *) _rt_key;
const char *s;
- const char *base; /* Base of heap */
herr_t ret_value = SUCCEED; /* Return value */
FUNC_ENTER_NOAPI_NOINIT
@@ -468,10 +462,6 @@ H5G_node_cmp3(void *_lt_key, void *_udata, void *_rt_key)
HDassert(lt_key);
HDassert(rt_key);
- /* Get base address of heap */
- base = (const char *)H5HL_offset_into(udata->heap, (size_t)0);
- HDassert(base);
-
/* left side */
if((s = (const char *)H5HL_offset_into(udata->heap, lt_key->offset)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get key name")
@@ -525,7 +515,6 @@ H5G_node_found(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void H5_ATTR_UNUSED
unsigned lt = 0, idx = 0, rt;
int cmp = 1;
const char *s;
- const char *base; /* Base of heap */
htri_t ret_value = TRUE; /* Return value */
FUNC_ENTER_NOAPI_NOINIT
@@ -543,10 +532,6 @@ H5G_node_found(H5F_t *f, hid_t dxpl_id, haddr_t addr, const void H5_ATTR_UNUSED
if(NULL == (sn = (H5G_node_t *)H5AC_protect(f, dxpl_id, H5AC_SNODE, addr, f, H5AC__READ_ONLY_FLAG)))
HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, FAIL, "unable to protect symbol table node")
- /* Get base address of heap */
- base = (const char *)H5HL_offset_into(udata->common.heap, (size_t)0);
- HDassert(base);
-
/*
* Binary search.
*/
@@ -624,7 +609,6 @@ H5G_node_insert(H5F_t *f, hid_t dxpl_id, haddr_t addr,
H5G_node_t *sn = NULL, *snrt = NULL;
unsigned sn_flags = H5AC__NO_FLAGS_SET, snrt_flags = H5AC__NO_FLAGS_SET;
const char *s;
- const char *base; /* Base of heap */
unsigned lt = 0, rt; /* Binary search cntrs */
int cmp = 1, idx = -1;
H5G_node_t *insert_into = NULL; /*node that gets new entry*/
@@ -649,10 +633,6 @@ H5G_node_insert(H5F_t *f, hid_t dxpl_id, haddr_t addr,
if(NULL == (sn = (H5G_node_t *)H5AC_protect(f, dxpl_id, H5AC_SNODE, addr, f, H5AC__NO_FLAGS_SET)))
HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, H5B_INS_ERROR, "unable to protect symbol table node")
- /* Get base address of heap */
- base = (const char *)H5HL_offset_into(udata->common.heap, (size_t)0);
- HDassert(base);
-
/*
* Where does the new symbol get inserted? We use a binary search.
*/
@@ -815,10 +795,6 @@ H5G_node_remove(H5F_t *f, hid_t dxpl_id, haddr_t addr, void *_lt_key/*in,out*/,
if(udata->common.name != NULL) {
H5O_link_t lnk; /* Constructed link for replacement */
size_t link_name_len; /* Length of string in local heap */
- const char *base; /* Base of heap */
-
- /* Get base address of heap */
- base = (const char *)H5HL_offset_into(udata->common.heap, (size_t)0);
/* Find the name with a binary search */
rt = sn->nsyms;
@@ -1019,7 +995,6 @@ H5G__node_iterate(H5F_t *f, hid_t dxpl_id, const void H5_ATTR_UNUSED *_lt_key, h
/* Get the pointer to the name of the link in the heap */
if((name = (const char *)H5HL_offset_into(udata->heap, ents[u].name_off)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, H5_ITER_ERROR, "unable to get symbol table node name")
- HDassert(name);
/* Convert the entry to a link */
if(H5G__ent_to_link(&lnk, udata->heap, &ents[u], name) < 0)
@@ -1362,7 +1337,6 @@ H5G__node_copy(H5F_t *f, hid_t dxpl_id, const void H5_ATTR_UNUSED *_lt_key, hadd
/* Determine name of source object */
if((name = (const char *)H5HL_offset_into(heap, src_ent->name_off)) == NULL)
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, H5_ITER_ERROR, "unable to get source object name")
- HDassert(name);
/* Set copied metadata tag */
H5_BEGIN_TAG(dxpl_id, H5AC__COPIED_TAG, H5_ITER_ERROR);
@@ -1447,7 +1421,6 @@ H5G__node_build_table(H5F_t *f, hid_t dxpl_id, const void H5_ATTR_UNUSED *_lt_ke
/* Get pointer to link's name in the heap */
if((name = (const char *)H5HL_offset_into(udata->heap, sn->entry[u].name_off)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, H5_ITER_ERROR, "unable to get symbol table link name")
- HDassert(name);
/* Determine the link to operate on in the table */
linkno = udata->ltable->nlinks++;
@@ -1549,29 +1522,29 @@ H5G_node_debug(H5F_t *f, hid_t dxpl_id, haddr_t addr, FILE * stream, int indent,
HGOTO_ERROR(H5E_SYM, H5E_CANTLOAD, FAIL, "unable to debug B-tree node");
} /* end if */
else {
- fprintf(stream, "%*sSymbol Table Node...\n", indent, "");
- fprintf(stream, "%*s%-*s %s\n", indent, "", fwidth,
+ HDfprintf(stream, "%*sSymbol Table Node...\n", indent, "");
+ HDfprintf(stream, "%*s%-*s %s\n", indent, "", fwidth,
"Dirty:",
sn->cache_info.is_dirty ? "Yes" : "No");
- fprintf(stream, "%*s%-*s %u\n", indent, "", fwidth,
+ HDfprintf(stream, "%*s%-*s %u\n", indent, "", fwidth,
"Size of Node (in bytes):", (unsigned)sn->node_size);
- fprintf(stream, "%*s%-*s %u of %u\n", indent, "", fwidth,
+ HDfprintf(stream, "%*s%-*s %u of %u\n", indent, "", fwidth,
"Number of Symbols:",
sn->nsyms, (unsigned)(2 * H5F_SYM_LEAF_K(f)));
indent += 3;
fwidth = MAX(0, fwidth - 3);
for(u = 0; u < sn->nsyms; u++) {
- fprintf(stream, "%*sSymbol %u:\n", indent - 3, "", u);
+ HDfprintf(stream, "%*sSymbol %u:\n", indent - 3, "", u);
if(heap) {
const char *s = (const char *)H5HL_offset_into(heap, sn->entry[u].name_off);
if(s)
- fprintf(stream, "%*s%-*s `%s'\n", indent, "", fwidth, "Name:", s);
+ HDfprintf(stream, "%*s%-*s `%s'\n", indent, "", fwidth, "Name:", s);
} /* end if */
else
- fprintf(stream, "%*s%-*s\n", indent, "", fwidth, "Warning: Invalid heap address given, name not displayed!");
+ HDfprintf(stream, "%*s%-*s\n", indent, "", fwidth, "Warning: Invalid heap address given, name not displayed!");
H5G__ent_debug(sn->entry + u, stream, indent, fwidth, heap);
} /* end for */
diff --git a/src/H5Gstab.c b/src/H5Gstab.c
index 7750671..4dc06ca 100644
--- a/src/H5Gstab.c
+++ b/src/H5Gstab.c
@@ -724,9 +724,8 @@ H5G_stab_get_name_by_idx_cb(const H5G_entry_t *ent, void *_udata)
if((name = (const char *)H5HL_offset_into(udata->heap, name_off)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get symbol table link name")
- HDassert(name);
- udata->name = H5MM_strdup(name);
- HDassert(udata->name);
+ if((udata->name = H5MM_strdup(name)) == NULL)
+ HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to duplicate symbol table link name")
done:
FUNC_LEAVE_NOAPI(ret_value)
@@ -948,7 +947,6 @@ H5G_stab_lookup_by_idx_cb(const H5G_entry_t *ent, void *_udata)
/* Get a pointer to the link name */
if((name = (const char *)H5HL_offset_into(udata->heap, ent->name_off)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "unable to get symbol table link name")
- HDassert(name);
/* Convert the entry to a link */
if(H5G__ent_to_link(udata->lnk, udata->heap, ent, name) < 0)
diff --git a/src/H5Oefl.c b/src/H5Oefl.c
index 2273289..7d78caf 100644
--- a/src/H5Oefl.c
+++ b/src/H5Oefl.c
@@ -151,7 +151,8 @@ H5O_efl_decode(H5F_t *f, hid_t dxpl_id, H5O_t H5_ATTR_UNUSED *open_oh,
if((s = (const char *)H5HL_offset_into(heap, mesg->slot[u].name_offset)) == NULL)
HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "unable to get external file name")
- HDassert(s && *s);
+ if(*s == NULL)
+ HGOTO_ERROR(H5E_SYM, H5E_CANTGET, NULL, "invalid external file name")
mesg->slot[u].name = H5MM_xstrdup (s);
HDassert(mesg->slot[u].name);
diff --git a/test/bad_offset.h5 b/test/bad_offset.h5
index 6500ffe..231dca2 100644
--- a/test/bad_offset.h5
+++ b/test/bad_offset.h5
Binary files differ
diff --git a/test/gen_bad_offset.c b/test/gen_bad_offset.c
index 2be4af1..82e94cd 100644
--- a/test/gen_bad_offset.c
+++ b/test/gen_bad_offset.c
@@ -27,9 +27,9 @@
/*-------------------------------------------------------------------------
* Function: main
- * Generate an HDF5 file with groups, datasets and symbolic links.
*
- * After this file is generated, write bad offset values to
+ * Generate an HDF5 file with groups, datasets and symbolic links.
+ * After the file is generated, write bad offset values to
* the heap at 3 locations in the file:
* (A) Open the file:
* fd = HDopen(TESTFILE, O_RDWR, 0663);
@@ -40,7 +40,7 @@
* "/dsetA": replace name offset into private heap "72" by bad offset
* (3) HDlseek(fd, (HDoff_t)1616, SEEK_SET);
* /soft_one: replace link value offset in the scratch pad "32" by bad offset
- * (C) Write the bad offset value to the file:
+ * (C) Write the bad offset value to the file for (1), (2) and (3):
* write(fd, &val, sizeof(val));
*
* Note: if the groups/datasets/symbolic links are changed in the file,
@@ -55,6 +55,8 @@ main(void)
{
hid_t fid = -1, gid1 = -1, gid2 = -1; /* File and group IDs */
hid_t did = -1, sid = -1; /* Dataset and dataspace IDs */
+ int fd = -1; /* File descriptor */
+ int64_t val = 999; /* Bad offset value */
/* Create the test file */
if((fid = H5Fcreate(TESTFILE, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
@@ -96,6 +98,39 @@ main(void)
if(H5Fclose(fid) < 0)
FAIL_STACK_ERROR
+ /*
+ * Write bad offset values at 3 locations in the file
+ */
+
+ /* Open the file */
+ if((fd = HDopen(TESTFILE, O_RDWR, 0663)) < 0)
+ FAIL_STACK_ERROR
+
+ /* Position the file for /group1/group2: replace heap offset "8" by bad offset */
+ if(HDlseek(fd, (HDoff_t)880, SEEK_SET) < 0)
+ FAIL_STACK_ERROR
+ /* Write the bad offset value to the file */
+ if(HDwrite(fd, &val, sizeof(val)) < 0)
+ FAIL_STACK_ERROR
+
+ /* Position the file for /dsetA: replace name offset into private heap "72" by bad offset */
+ if(HDlseek(fd, (HDoff_t)1512, SEEK_SET) < 0)
+ FAIL_STACK_ERROR
+ /* Write the bad offset value to the file */
+ if(HDwrite(fd, &val, sizeof(val)) < 0)
+ FAIL_STACK_ERROR
+
+ /* Position the file for /soft_one: replace link value offset in the scratch pad "32" by bad offset */
+ if(HDlseek(fd, (HDoff_t)1616, SEEK_SET) < 0)
+ FAIL_STACK_ERROR
+ /* Write the bad offset value to the file */
+ if(HDwrite(fd, &val, sizeof(val)) < 0)
+ FAIL_STACK_ERROR
+
+ /* Close the file */
+ if(HDclose(fd) < 0)
+ FAIL_STACK_ERROR
+
return EXIT_SUCCESS;
error:
diff --git a/test/tmisc.c b/test/tmisc.c
index 594a513..2c0fa96 100644
--- a/test/tmisc.c
+++ b/test/tmisc.c
@@ -5547,7 +5547,7 @@ test_misc(void)
test_misc30(); /* Exercise local heap loading bug where free lists were getting dropped */
test_misc31(); /* Test Reentering library through deprecated routines after H5close() */
test_misc32(); /* Test filter memory allocation functions */
- test_misc33(); /* ??? */
+ test_misc33(); /* Test to verify that H5HL_offset_into() returns error if offset exceeds heap block */
} /* test_misc() */