From 48b4a56d934088305d710358405aa44f4af9eaf7 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Wed, 8 Oct 2003 12:58:58 -0500 Subject: [svn-r7572] Purpose: Bug fix Description: When too many messages were inserted into an object header, the library had an internal pointer to the "new message" that was pointing to the incorrect location when the array of messages was re-allocated. In the worst case, this could cause a file to be corrupted. Solution: Update the internal pointer when the array is re-allocated. Platforms tested: FreeBSD 4.9 (sleipnir) Too small to require h5committest --- release_docs/RELEASE.txt | 6 ++++- src/H5O.c | 65 ++++++++++++++++++++++++++---------------------- test/ohdr.c | 41 +++++++++++++++++++++++------- 3 files changed, 72 insertions(+), 40 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index d5f1d95..8fd5a7d 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -80,8 +80,12 @@ Bug Fixes since HDF5-1.6.0 release Library ------- + - Fixed potential file corruption bug when too many object header + messages (probably attributes, from a user perspective) were + inserted into an object header and certain other conditions were + met. QAK - 2003/10/08 - Changed implementation of internal ID searching algorithm to avoid - O(n) behavior for many common cases. QAK 2003/10/06 + O(n) behavior for many common cases. QAK - 2003/10/06 - Allow partial parallel writing to compact datasets. QAK - 2003/10/06 - Correctly create reference to shared datatype in attribute, instead of making a copy of the shared datatype in the attribute. diff --git a/src/H5O.c b/src/H5O.c index ebda246..48c0751 100644 --- a/src/H5O.c +++ b/src/H5O.c @@ -2571,36 +2571,38 @@ H5O_alloc_extend_chunk(H5O_t *oh, unsigned chunkno, size_t size) /* try to extend a null message */ for (idx=0; idxnmesgs; idx++) { - if (H5O_NULL_ID == oh->mesg[idx].type->id && - (oh->mesg[idx].raw + oh->mesg[idx].raw_size == - oh->chunk[chunkno].image + oh->chunk[chunkno].size)) { - - delta = MAX (H5O_MIN_SIZE, aligned_size - oh->mesg[idx].raw_size); - assert (delta=H5O_ALIGN (delta)); - oh->mesg[idx].dirty = TRUE; - oh->mesg[idx].raw_size += delta; - - old_addr = oh->chunk[chunkno].image; - - /* Be careful not to indroduce garbage */ - oh->chunk[chunkno].image = H5FL_BLK_REALLOC(chunk_image,old_addr, - (oh->chunk[chunkno].size + delta)); - if (NULL==oh->chunk[chunkno].image) - HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, UFAIL, "memory allocation failed"); - HDmemset(oh->chunk[chunkno].image + oh->chunk[chunkno].size, - 0, delta); - oh->chunk[chunkno].size += delta; - - /* adjust raw addresses for messages of this chunk */ - if (old_addr != oh->chunk[chunkno].image) { - for (u = 0; u < oh->nmesgs; u++) { - if (oh->mesg[u].chunkno == chunkno) - oh->mesg[u].raw = oh->chunk[chunkno].image + - (oh->mesg[u].raw - old_addr); - } - } - HGOTO_DONE(idx); - } + if (oh->mesg[idx].chunkno==chunkno) { + if ( H5O_NULL_ID == oh->mesg[idx].type->id && + (oh->mesg[idx].raw + oh->mesg[idx].raw_size == + oh->chunk[chunkno].image + oh->chunk[chunkno].size)) { + + delta = MAX (H5O_MIN_SIZE, aligned_size - oh->mesg[idx].raw_size); + assert (delta=H5O_ALIGN (delta)); + oh->mesg[idx].dirty = TRUE; + oh->mesg[idx].raw_size += delta; + + old_addr = oh->chunk[chunkno].image; + + /* Be careful not to indroduce garbage */ + oh->chunk[chunkno].image = H5FL_BLK_REALLOC(chunk_image,old_addr, + (oh->chunk[chunkno].size + delta)); + if (NULL==oh->chunk[chunkno].image) + HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, UFAIL, "memory allocation failed"); + HDmemset(oh->chunk[chunkno].image + oh->chunk[chunkno].size, + 0, delta); + oh->chunk[chunkno].size += delta; + + /* adjust raw addresses for messages of this chunk */ + if (old_addr != oh->chunk[chunkno].image) { + for (u = 0; u < oh->nmesgs; u++) { + if (oh->mesg[u].chunkno == chunkno) + oh->mesg[u].raw = oh->chunk[chunkno].image + + (oh->mesg[u].raw - old_addr); + } + } + HGOTO_DONE(idx); + } + } /* end if */ } /* create a new null message */ @@ -2945,6 +2947,9 @@ H5O_alloc(H5F_t *f, H5O_t *oh, const H5O_class_t *type, size_t size) /* Set new object header info to zeros */ HDmemset(&oh->mesg[old_alloc],0, (oh->alloc_nmesgs-old_alloc)*sizeof(H5O_mesg_t)); + + /* "Retarget" local 'msg' pointer into newly allocated array of messages */ + msg=&oh->mesg[idx]; } null_msg=&oh->mesg[oh->nmesgs++]; null_msg->type = H5O_NULL; diff --git a/test/ohdr.c b/test/ohdr.c index c706b4b..5ddabe6 100644 --- a/test/ohdr.c +++ b/test/ohdr.c @@ -101,7 +101,7 @@ main(void) /* create a new message */ TESTING("message creation"); time_new = 11111111; - if (H5O_modify(&oh_ent, H5O_MTIME_ID, H5O_NEW_MESG, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { + if (H5O_modify(&oh_ent, H5O_MTIME_NEW_ID, H5O_NEW_MESG, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -119,7 +119,7 @@ main(void) #endif /* H5_WANT_H5_V1_6_COMPAT */ goto error; } - if (NULL==H5O_read(&oh_ent, H5O_MTIME_ID, 0, &ro, H5P_DATASET_XFER_DEFAULT)) { + if (NULL==H5O_read(&oh_ent, H5O_MTIME_NEW_ID, 0, &ro, H5P_DATASET_XFER_DEFAULT)) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -141,7 +141,7 @@ main(void) */ TESTING("message modification"); time_new = 33333333; - if (H5O_modify(&oh_ent, H5O_MTIME_ID, 0, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { + if (H5O_modify(&oh_ent, H5O_MTIME_NEW_ID, 0, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -159,7 +159,7 @@ main(void) #endif /* H5_WANT_H5_V1_6_COMPAT */ goto error; } - if (NULL==H5O_read(&oh_ent, H5O_MTIME_ID, 0, &ro, H5P_DATASET_XFER_DEFAULT)) { + if (NULL==H5O_read(&oh_ent, H5O_MTIME_NEW_ID, 0, &ro, H5P_DATASET_XFER_DEFAULT)) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -182,7 +182,7 @@ main(void) */ TESTING("duplicate message creation"); time_new = 55555555; - if (H5O_modify(&oh_ent, H5O_MTIME_ID, H5O_NEW_MESG, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { + if (H5O_modify(&oh_ent, H5O_MTIME_NEW_ID, H5O_NEW_MESG, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -200,7 +200,7 @@ main(void) #endif /* H5_WANT_H5_V1_6_COMPAT */ goto error; } - if (NULL==H5O_read(&oh_ent, H5O_MTIME_ID, 1, &ro, H5P_DATASET_XFER_DEFAULT)) { + if (NULL==H5O_read(&oh_ent, H5O_MTIME_NEW_ID, 1, &ro, H5P_DATASET_XFER_DEFAULT)) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -222,7 +222,7 @@ main(void) */ TESTING("duplicate message modification"); time_new = 77777777; - if (H5O_modify(&oh_ent, H5O_MTIME_ID, 1, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { + if (H5O_modify(&oh_ent, H5O_MTIME_NEW_ID, 1, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -240,7 +240,7 @@ main(void) #endif /* H5_WANT_H5_V1_6_COMPAT */ goto error; } - if (NULL==H5O_read(&oh_ent, H5O_MTIME_ID, 1, &ro, H5P_DATASET_XFER_DEFAULT)) { + if (NULL==H5O_read(&oh_ent, H5O_MTIME_NEW_ID, 1, &ro, H5P_DATASET_XFER_DEFAULT)) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -260,6 +260,10 @@ main(void) /* * Test creation of a bunch of messages one after another to see * what happens when the object header overflows in core. + * (Use 'old' MTIME message here, because it is large enough to be + * replaced with a continuation message (the new one is too small) + * and the library doesn't understand how to migrate more than one + * message from an object header currently - QAK - 10/8/03) */ TESTING("object header overflow in memory"); for (i=0; i<40; i++) { @@ -292,7 +296,7 @@ main(void) TESTING("object header overflow on disk"); for (i=0; i<10; i++) { time_new = (i + 1) * 1000 + 10; - if (H5O_modify(&oh_ent, H5O_MTIME_ID, H5O_NEW_MESG, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { + if (H5O_modify(&oh_ent, H5O_MTIME_NEW_ID, H5O_NEW_MESG, 0, 0, &time_new, H5P_DATASET_XFER_DEFAULT)<0) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT H5Eprint(stdout); @@ -317,6 +321,15 @@ main(void) * Delete all time messages. */ TESTING("message deletion"); + if (H5O_remove(&oh_ent, H5O_MTIME_NEW_ID, H5O_ALL, H5P_DATASET_XFER_DEFAULT)<0) { + H5_FAILED(); +#ifdef H5_WANT_H5_V1_6_COMPAT + H5Eprint(stdout); +#else + H5Eprint(H5E_DEFAULT, stdout); +#endif /* H5_WANT_H5_V1_6_COMPAT */ + goto error; + } if (H5O_remove(&oh_ent, H5O_MTIME_ID, H5O_ALL, H5P_DATASET_XFER_DEFAULT)<0) { H5_FAILED(); #ifdef H5_WANT_H5_V1_6_COMPAT @@ -326,6 +339,16 @@ main(void) #endif /* H5_WANT_H5_V1_6_COMPAT */ goto error; } + if (H5O_read(&oh_ent, H5O_MTIME_NEW_ID, 0, &ro, H5P_DATASET_XFER_DEFAULT)) { + H5_FAILED(); + puts(" H5O_read() should have failed but didn't"); +#ifdef H5_WANT_H5_V1_6_COMPAT + H5Eclear(); +#else + H5Eclear(H5E_DEFAULT); +#endif /* H5_WANT_H5_V1_6_COMPAT */ + goto error; + } if (H5O_read(&oh_ent, H5O_MTIME_ID, 0, &ro, H5P_DATASET_XFER_DEFAULT)) { H5_FAILED(); puts(" H5O_read() should have failed but didn't"); -- cgit v0.12