From 23fca0ce4be0217f982c155d6239c48c85deb594 Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Thu, 29 Feb 2024 15:50:22 -0600 Subject: [PATCH] Fix selection when using vlen types --- src/rest_vol.c | 58 ++++++++++- src/rest_vol.h | 6 +- src/rest_vol_dataset.c | 217 ++++++++++++++++++++++------------------- 3 files changed, 178 insertions(+), 103 deletions(-) diff --git a/src/rest_vol.c b/src/rest_vol.c index 5c37d8c8..59e2e5c6 100644 --- a/src/rest_vol.c +++ b/src/rest_vol.c @@ -3635,6 +3635,8 @@ RV_curl_multi_perform(CURL *curl_multi_handle, dataset_transfer_info *transfer_i fail_count++; } else if (response_code == 200) { + H5T_class_t dtype_class = H5T_NO_CLASS; + num_finished++; succeed_count++; @@ -3661,8 +3663,60 @@ RV_curl_multi_perform(CURL *curl_multi_handle, dataset_transfer_info *transfer_i transfer_info[handle_index].curl_easy_handle = NULL; if (transfer_info[handle_index].transfer_type == WRITE) { - RV_free(transfer_info[handle_index].u.write_info.alloc_write_buffer); - transfer_info[handle_index].u.write_info.alloc_write_buffer = NULL; + if (transfer_info[handle_index].tconv_buf) { + if ((dtype_class = H5Tget_class(transfer_info[handle_index].mem_type_id)) == + H5T_NO_CLASS) + FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get mem dtype class"); + + /* Clean up memory allocated by type conversion of vlen types */ + if (dtype_class == H5T_VLEN) { + /* Buffer was gathered before type conversion, so we can manually free vlen + * memory by iteration */ + hssize_t num_elems = 0; + if ((num_elems = H5Sget_select_npoints( + transfer_info[handle_index].mem_space_id)) <= 0) + FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, + "can't get number of elements in dataspace"); + + hvl_t *vlen_buf = (hvl_t *)transfer_info[handle_index].tconv_buf; + for (size_t i = 0; i < (size_t)num_elems; i++) { + if (vlen_buf[i].p) { + RV_free(vlen_buf[i].p); + vlen_buf[i].p = NULL; + } + } + } + } + + if (transfer_info[handle_index].u.write_info.gather_buf) { + RV_free(transfer_info[handle_index].u.write_info.gather_buf); + transfer_info[handle_index].u.write_info.gather_buf = NULL; + } + + if (transfer_info[handle_index].u.write_info.serialize_buf) { + RV_free(transfer_info[handle_index].u.write_info.serialize_buf); + transfer_info[handle_index].u.write_info.serialize_buf = NULL; + } + + if (transfer_info[handle_index].u.write_info.base64_encoded_values) { + RV_free(transfer_info[handle_index].u.write_info.base64_encoded_values); + transfer_info[handle_index].u.write_info.base64_encoded_values = NULL; + } + + if (transfer_info[handle_index].u.write_info.selection_buf) { + RV_free(transfer_info[handle_index].u.write_info.selection_buf); + transfer_info[handle_index].u.write_info.selection_buf = NULL; + } + } + + if (transfer_info[handle_index].tconv_buf) { + RV_free(transfer_info[handle_index].tconv_buf); + transfer_info[handle_index].tconv_buf = NULL; + } + + if (transfer_info[handle_index].bkg_buf) { + RV_free(transfer_info[handle_index].bkg_buf); + transfer_info[handle_index].bkg_buf = NULL; } RV_free(transfer_info[handle_index].request_url); diff --git a/src/rest_vol.h b/src/rest_vol.h index 61ed2d3b..ae3f6a1d 100644 --- a/src/rest_vol.h +++ b/src/rest_vol.h @@ -570,7 +570,10 @@ struct RV_object_t { /* Structures to hold information for cURL requests to read/write to datasets */ typedef struct dataset_write_info { /* This points to the dynamically allocated buffer for write, if any */ - char *alloc_write_buffer; + void *gather_buf; + void *base64_encoded_values; + void *serialize_buf; + void *selection_buf; upload_info uinfo; } dataset_write_info; @@ -600,6 +603,7 @@ typedef struct dataset_transfer_info { char *selection_body; /* Fields for type conversion */ + void *tconv_buf; void *bkg_buf; transfer_type_t transfer_type; diff --git a/src/rest_vol_dataset.c b/src/rest_vol_dataset.c index d2352a26..cbd8b05d 100644 --- a/src/rest_vol_dataset.c +++ b/src/rest_vol_dataset.c @@ -804,7 +804,7 @@ RV_dataset_read(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spac "failed to set max concurrent streams for curl multi handle"); if (RV_curl_multi_perform(curl_multi_handle, transfer_info, count, rv_dataset_read_cb) < 0) - FUNC_GOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "failed to perform dataset write"); + FUNC_GOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "failed to perform dataset read"); done: @@ -877,10 +877,6 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa size_t mem_type_size = 0; hbool_t fill_bkg = FALSE; - /* Temporary pointer for reallocations of write body */ - char *temp_ptr = NULL; - char *tconv_buf = NULL; - if ((transfer_info = RV_calloc(count * sizeof(dataset_transfer_info))) == NULL) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "can't allocate space for dataset transfer info"); @@ -925,10 +921,9 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't set up non global curl write data: %s", transfer_info[i].curl_err_buf); - transfer_info[i].u.write_info.alloc_write_buffer = NULL; - transfer_info[i].dataset = (RV_object_t *)dset[i]; - transfer_info[i].buf = (void *)buf[i]; - transfer_info[i].transfer_type = WRITE; + transfer_info[i].dataset = (RV_object_t *)dset[i]; + transfer_info[i].buf = (void *)buf[i]; + transfer_info[i].transfer_type = WRITE; transfer_info[i].mem_space_id = _mem_space_id[i]; transfer_info[i].file_space_id = _file_space_id[i]; @@ -938,11 +933,16 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa transfer_info[i].host_headers = NULL; transfer_info[i].resp_buffer.buffer_size = CURL_RESPONSE_BUFFER_DEFAULT_SIZE; transfer_info[i].resp_buffer.curr_buf_ptr = transfer_info[i].resp_buffer.buffer; + transfer_info[i].tconv_buf = NULL; transfer_info[i].bkg_buf = NULL; - transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].buf; - transfer_info[i].u.write_info.uinfo.buffer_size = 0; - transfer_info[i].u.write_info.uinfo.bytes_sent = 0; + transfer_info[i].u.write_info.gather_buf = NULL; + transfer_info[i].u.write_info.serialize_buf = NULL; + transfer_info[i].u.write_info.base64_encoded_values = NULL; + transfer_info[i].u.write_info.selection_buf = NULL; + transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].buf; + transfer_info[i].u.write_info.uinfo.buffer_size = 0; + transfer_info[i].u.write_info.uinfo.bytes_sent = 0; } #ifdef RV_CONNECTOR_DEBUG @@ -1022,7 +1022,8 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa is_transfer_binary = is_transfer_binary && (H5S_SEL_POINTS != sel_type); if (RV_convert_dataspace_selection_to_string(transfer_info[i].file_space_id, &selection_body, - &selection_body_len, is_transfer_binary) < 0) + &selection_body_len, + (dtype_class == H5T_VLEN) || is_transfer_binary) < 0) FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTCONVERT, FAIL, "can't convert dataspace selection to string representation"); } /* end else */ @@ -1054,15 +1055,14 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa "Unable to determine if the dataspace selection is contiguous"); if (!contiguous) { - if (NULL == - (transfer_info[i].u.write_info.alloc_write_buffer = (char *)RV_malloc(write_body_len))) + if (NULL == (transfer_info[i].u.write_info.gather_buf = (char *)RV_malloc(write_body_len))) FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, "can't allocate space for the 'write_body' values"); if (H5Dgather(transfer_info[i].mem_space_id, transfer_info[i].u.write_info.uinfo.buffer, transfer_info[i].file_type_id, write_body_len, - transfer_info[i].u.write_info.alloc_write_buffer, NULL, NULL) < 0) + transfer_info[i].u.write_info.gather_buf, NULL, NULL) < 0) FUNC_GOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "can't gather data to write buffer"); - transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.alloc_write_buffer; + transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.gather_buf; } /* Handle conversion from memory datatype to file datatype, if necessary */ @@ -1076,27 +1076,21 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa #endif /* Initialize type conversion */ RV_tconv_init(transfer_info[i].mem_type_id, &mem_type_size, transfer_info[i].file_type_id, - &file_type_size, (size_t)file_select_npoints, TRUE, FALSE, (void **)&tconv_buf, - &transfer_info[i].bkg_buf, NULL, &fill_bkg); + &file_type_size, (size_t)file_select_npoints, TRUE, FALSE, + (void **)&transfer_info[i].tconv_buf, &transfer_info[i].bkg_buf, NULL, &fill_bkg); /* Perform type conversion on response values */ - memset(tconv_buf, 0, file_type_size * (size_t)mem_select_npoints); - memcpy(tconv_buf, transfer_info[i].u.write_info.uinfo.buffer, + memset(transfer_info[i].tconv_buf, 0, file_type_size * (size_t)mem_select_npoints); + memcpy(transfer_info[i].tconv_buf, transfer_info[i].u.write_info.uinfo.buffer, mem_type_size * (size_t)mem_select_npoints); if (H5Tconvert(transfer_info[i].mem_type_id, transfer_info[i].file_type_id, - (size_t)file_select_npoints, tconv_buf, transfer_info[i].bkg_buf, H5P_DEFAULT) < 0) + (size_t)file_select_npoints, transfer_info[i].tconv_buf, transfer_info[i].bkg_buf, + H5P_DEFAULT) < 0) FUNC_GOTO_ERROR(H5E_DATATYPE, H5E_CANTCONVERT, FAIL, "failed to convert file datatype to memory datatype"); - if (transfer_info[i].u.write_info.alloc_write_buffer) { - RV_free(transfer_info[i].u.write_info.alloc_write_buffer); - transfer_info[i].u.write_info.alloc_write_buffer = NULL; - } - - transfer_info[i].u.write_info.alloc_write_buffer = tconv_buf; - transfer_info[i].u.write_info.uinfo.buffer = tconv_buf; - tconv_buf = NULL; + transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].tconv_buf; } /* Setup the size of the data being transferred and the data buffer itself (for non-simple @@ -1114,27 +1108,24 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa } /* end if */ else { if (H5T_VLEN == dtype_class) { - if (transfer_info[i].u.write_info.alloc_write_buffer) { - RV_free(transfer_info[i].u.write_info.alloc_write_buffer); - transfer_info[i].u.write_info.alloc_write_buffer = NULL; - } - if (RV_vlen_data_to_json(transfer_info[i].u.write_info.uinfo.buffer, (size_t)file_select_npoints, mem_type_id[0], - (void **)&transfer_info[i].u.write_info.alloc_write_buffer, + (void **)&transfer_info[i].u.write_info.serialize_buf, &write_body_len) < 0) FUNC_GOTO_ERROR(H5E_DATATYPE, H5E_CANTCONVERT, FAIL, "can't convert vlen data to a buffer"); + + transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.serialize_buf; } else if (H5T_STD_REF_OBJ == transfer_info[i].file_type_id) { /* Convert the buffer of rest_obj_ref_t's to a binary buffer */ if (RV_convert_obj_refs_to_buffer( (const rv_obj_ref_t *)transfer_info[i].u.write_info.uinfo.buffer, - (size_t)file_select_npoints, &(transfer_info[i].u.write_info.alloc_write_buffer), + (size_t)file_select_npoints, (char **)&(transfer_info[i].u.write_info.serialize_buf), &write_body_len) < 0) FUNC_GOTO_ERROR(H5E_DATATYPE, H5E_CANTCONVERT, FAIL, "can't convert object ref/s to ref string/s"); - transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.alloc_write_buffer; + transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.serialize_buf; } /* end if */ } /* end else */ @@ -1160,12 +1151,17 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa is_transfer_binary ? "Content-Type: application/octet-stream" : "Content-Type: application/json"); /* Redirect cURL from the base URL to "/datasets//value" to write the value out */ - if ((url_len = snprintf( - transfer_info[i].request_url, URL_MAX_LENGTH, "%s/datasets/%s/value%s%s", - transfer_info[i].dataset->domain->u.file.server_info.base_URL, transfer_info[i].dataset->URI, - is_transfer_binary && selection_body && (H5S_SEL_POINTS != sel_type) ? "?select=" : "", - is_transfer_binary && selection_body && (H5S_SEL_POINTS != sel_type) ? selection_body - : "")) < 0) + if ((url_len = snprintf(transfer_info[i].request_url, URL_MAX_LENGTH, "%s/datasets/%s/value%s%s", + transfer_info[i].dataset->domain->u.file.server_info.base_URL, + transfer_info[i].dataset->URI, + (is_transfer_binary || dtype_class == H5T_VLEN) && selection_body && + (H5S_SEL_POINTS != sel_type) + ? "?select=" + : "", + (is_transfer_binary || dtype_class == H5T_VLEN) && selection_body && + (H5S_SEL_POINTS != sel_type) + ? selection_body + : "")) < 0) FUNC_GOTO_ERROR(H5E_DATASET, H5E_SYSERRSTR, FAIL, "snprintf error"); if (url_len >= URL_MAX_LENGTH) @@ -1176,10 +1172,10 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa printf("-> Dataset write URL: %s\n\n", transfer_info[0].request_url); #endif - /* If using a point selection, add the selection body + /* If using a point selection, or doing a selection with a JSON write, add the selection body * into the write body sent to server. */ - if (H5S_SEL_POINTS == sel_type) { + if ((H5S_SEL_POINTS == sel_type)) { const char *const fmt_string = "{%s,\"value_base64\": \"%s\"}"; size_t value_body_len; int bytes_printed; @@ -1190,43 +1186,33 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa */ value_body_len = (size_t)((4.0 / 3.0) * (double)write_body_len); - temp_ptr = transfer_info[i].u.write_info.alloc_write_buffer; - - if (NULL == (transfer_info[i].u.write_info.alloc_write_buffer = RV_malloc(value_body_len))) + if (NULL == (transfer_info[i].u.write_info.base64_encoded_values = RV_malloc(value_body_len))) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "can't allocate temporary buffer for base64-encoded write buffer"); if (RV_base64_encode(transfer_info[i].u.write_info.uinfo.buffer, write_body_len, - &transfer_info[i].u.write_info.alloc_write_buffer, &value_body_len) < 0) + (char **)&transfer_info[i].u.write_info.base64_encoded_values, + &value_body_len) < 0) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTENCODE, FAIL, "can't base64-encode write buffer"); - /* If previous write body was allocated on the heap, free it */ - if (transfer_info[i].u.write_info.uinfo.buffer == temp_ptr) { - RV_free(temp_ptr); - temp_ptr = NULL; - } - - transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.alloc_write_buffer; - #ifdef RV_CONNECTOR_DEBUG printf("-> Base64-encoded data buffer for dataset %zu: %s\n\n", i, - transfer_info[i].u.write_info.write_body); + transfer_info[i].u.write_info.base64_encoded_values); #endif /* Copy encoded values into format string */ - temp_ptr = transfer_info[i].u.write_info.alloc_write_buffer; - write_body_len = (strlen(fmt_string) - 4) + selection_body_len + value_body_len; - if (NULL == (transfer_info[i].u.write_info.alloc_write_buffer = RV_malloc(write_body_len + 1))) + if (NULL == (transfer_info[i].u.write_info.selection_buf = RV_malloc(write_body_len + 1))) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "can't allocate space for write buffer"); - if ((bytes_printed = snprintf(transfer_info[i].u.write_info.alloc_write_buffer, - write_body_len + 1, fmt_string, selection_body, temp_ptr)) < 0) + if ((bytes_printed = + snprintf(transfer_info[i].u.write_info.selection_buf, write_body_len + 1, fmt_string, + selection_body, transfer_info[i].u.write_info.base64_encoded_values)) < 0) FUNC_GOTO_ERROR(H5E_DATASET, H5E_SYSERRSTR, FAIL, "snprintf error"); - transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.alloc_write_buffer; + transfer_info[i].u.write_info.uinfo.buffer = transfer_info[i].u.write_info.selection_buf; #ifdef RV_CONNECTOR_DEBUG - printf("-> Write body: %s\n\n", transfer_info[i].u.write_info.write_body); + printf("-> Write body: %s\n\n", transfer_info[i].u.write_info.selection_buf); #endif if (bytes_printed >= write_body_len + 1) @@ -1236,13 +1222,10 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa transfer_info[i].curl_headers = curl_slist_append(transfer_info[i].curl_headers, "Content-Type: application/json"); - RV_free(temp_ptr); - temp_ptr = NULL; - #ifdef RV_CONNECTOR_DEBUG printf("-> Setup cURL to POST point list for dataset write\n\n"); #endif - } /* end if */ + } transfer_info[i].u.write_info.uinfo.buffer_size = write_body_len; transfer_info[i].u.write_info.uinfo.bytes_sent = 0; @@ -1287,10 +1270,6 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa selection_body = NULL; } - if (tconv_buf) { - RV_free(tconv_buf); - tconv_buf = NULL; - } } /* End iteration over dsets to write to */ #ifdef RV_CONNECTOR_DEBUG @@ -1325,19 +1304,39 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa curl_easy_cleanup(transfer_info[i].curl_easy_handle); } - RV_free(transfer_info[i].u.write_info.alloc_write_buffer); - RV_free(transfer_info[i].request_url); - RV_free(transfer_info[i].resp_buffer.buffer); + if (transfer_info[i].tconv_buf) { + /* Clean up memory allocated by type conversion of vlen types */ + if ((dtype_class = H5Tget_class(transfer_info[i].mem_type_id)) == H5T_NO_CLASS) + FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get mem dtype class"); - /* Clean up memory allocated by type conversion of vlen types */ - if ((dtype_class = H5Tget_class(transfer_info[i].mem_type_id)) == H5T_NO_CLASS) - FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get mem dtype class"); + if (dtype_class == H5T_VLEN) { + /* Type conversion buffer is packed, so free allocated buffers manually */ + for (size_t j = 0; j < (size_t)file_select_npoints; j++) { + hvl_t vl = ((hvl_t *)transfer_info[i].tconv_buf)[j]; - if (dtype_class == H5T_VLEN) - if (H5Treclaim(transfer_info[i].mem_type_id, transfer_info[i].mem_space_id, H5P_DEFAULT, - transfer_info[i].u.write_info.alloc_write_buffer) < 0) - FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, - "can't reclaim vlen memory allocated by conversion"); + if (vl.p) + RV_free(vl.p); + } + } + + RV_free(transfer_info[i].tconv_buf); + transfer_info[i].tconv_buf = NULL; + } + + if (transfer_info[i].u.write_info.gather_buf) + RV_free(transfer_info[i].u.write_info.gather_buf); + + if (transfer_info[i].u.write_info.serialize_buf) + RV_free(transfer_info[i].u.write_info.serialize_buf); + + if (transfer_info[i].u.write_info.base64_encoded_values) + RV_free(transfer_info[i].u.write_info.base64_encoded_values); + + if (transfer_info[i].u.write_info.selection_buf) + RV_free(transfer_info[i].u.write_info.selection_buf); + + RV_free(transfer_info[i].request_url); + RV_free(transfer_info[i].resp_buffer.buffer); if (transfer_info[i].bkg_buf) RV_free(transfer_info[i].bkg_buf); @@ -1346,10 +1345,6 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa RV_free(transfer_info[i].host_headers); } - if (temp_ptr) { - RV_free(temp_ptr); - } - curl_multi_cleanup(curl_multi_handle); RV_free(transfer_info); @@ -5011,8 +5006,26 @@ RV_json_values_to_binary_callback(char *HTTP_response, void *callback_data_in, v if (ret_value >= 0 && value_buffer) *out_buf = value_buffer; - if (ret_value < 0 && value_buffer) + if (ret_value < 0 && value_buffer) { + H5T_class_t dtype_class = H5T_NO_CLASS; + + if ((dtype_class = H5Tget_class(dtype_id)) < 0) + FUNC_DONE_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get datatype class"); + + if (dtype_class == H5T_VLEN) { + /* Reclaim each allocated vlen buffer */ + hvl_t *curr = (hvl_t *)value_buffer; + + while (curr->p) { + RV_free(curr->p); + curr->p = NULL; + curr += 1; + } + } + RV_free(value_buffer); + *out_buf = NULL; + } return ret_value; } @@ -5025,6 +5038,8 @@ RV_json_values_to_binary_recursive(yajl_val value_entry, hid_t dtype_id, void *v H5T_class_t dtype_class = H5T_NO_CLASS; size_t dtype_size = 0; + hvl_t *vl = NULL; + if ((dtype_size = H5Tget_size(dtype_id)) <= 0) FUNC_GOTO_ERROR(H5E_DATATYPE, H5E_CANTGET, FAIL, "can't get datatype size"); @@ -5113,9 +5128,15 @@ RV_json_values_to_binary_recursive(yajl_val value_entry, hid_t dtype_id, void *v case (H5T_VLEN): { yajl_val seq_elem_val = NULL; hid_t parent_dtype_id = H5I_INVALID_HID; - hvl_t *vl = ((hvl_t *)value_buffer); size_t parent_dtype_size = 0; void *seq_elem_ptr = NULL; + vl = (hvl_t *)value_buffer; + + if ((parent_dtype_id = H5Tget_super(dtype_id)) == H5I_INVALID_HID) + FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get base type of vlen sequence"); + + if ((parent_dtype_size = H5Tget_size(parent_dtype_id)) == 0) + FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get size of vlen parent type"); if (!YAJL_IS_ARRAY(value_entry)) FUNC_GOTO_ERROR(H5E_DATASET, H5E_PARSEERROR, FAIL, @@ -5128,21 +5149,15 @@ RV_json_values_to_binary_recursive(yajl_val value_entry, hid_t dtype_id, void *v FUNC_GOTO_ERROR(H5E_DATASET, H5E_PARSEERROR, FAIL, "server returned vlen sequence with invalid length"); - if ((vl->p = calloc(vl->len, sizeof(void *))) == NULL) + if ((vl->p = calloc(vl->len, parent_dtype_size)) == NULL) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "can't allocate space for returned vlen seq data"); /* Recursively encode each element in the sequence and place it in the allocated buffer */ for (size_t i = 0; i < vl->len; i++) { - if ((parent_dtype_id = H5Tget_super(dtype_id)) == H5I_INVALID_HID) - FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get base type of vlen sequence"); - if ((seq_elem_val = YAJL_GET_ARRAY(value_entry)->values[i]) == NULL) FUNC_GOTO_ERROR(H5E_DATASET, H5E_PARSEERROR, FAIL, "can't parse vlen sequence element"); - if ((parent_dtype_size = H5Tget_size(parent_dtype_id)) == 0) - FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTGET, FAIL, "can't get size of vlen parent type"); - seq_elem_ptr = (void *)(((char *)vl->p) + i * parent_dtype_size); if (RV_json_values_to_binary_recursive(seq_elem_val, parent_dtype_id, seq_elem_ptr) < 0) @@ -5159,8 +5174,10 @@ RV_json_values_to_binary_recursive(yajl_val value_entry, hid_t dtype_id, void *v done: if ((ret_value < 0) && (dtype_class == H5T_VLEN)) { - if (((hvl_t *)value_buffer)->p) - RV_free(((hvl_t *)value_buffer)->p); + if (vl->p) { + RV_free(vl->p); + vl->p = NULL; + } } return ret_value;