Skip to content

Commit 72f52cc

Browse files
committed
HL: address PR #6140 review comments
H5LT.c: - Push H5E_NOSPACE on strdup OOM in H5LTtext_to_dtype so the HDF5 error stack is populated on allocation failure - Rename static helper H5LT_append_dtype_super_text -> append_dtype_super_text to match the TU-local naming convention (no H5LT_ prefix for file-static helpers); update all four call sites - Add comment on super_len == 0 guard explaining calloc(0,...) avoidance - Push H5E_NOSPACE from calloc failure inside the helper H5TB.c (H5TBget_field_info): - Clarify else-branch comment: the two-branch structure is an efficiency optimization (copy only name_len+1 bytes rather than HLTB_MAX_FIELD_LEN-1), not a backward-compatibility concern (addresses jhendersonHDF review note)
1 parent b5be881 commit 72f52cc

2 files changed

Lines changed: 16 additions & 12 deletions

File tree

hl/src/H5LT.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1889,8 +1889,9 @@ H5LTtext_to_dtype(const char *text, H5LT_lang_t lang_type)
18891889

18901890
input_len = strlen(text);
18911891
myinput = strdup(text);
1892-
if (!myinput)
1893-
goto out;
1892+
if (!myinput) {
1893+
H5Epush_ret(__func__, H5E_ERR_CLS, H5E_RESOURCE, H5E_NOSPACE, "memory allocation failed", -1);
1894+
}
18941895

18951896
if ((type_id = H5LTyyparse()) < 0) {
18961897
free(myinput);
@@ -2189,7 +2190,7 @@ H5LTdtype_to_text(hid_t dtype, char *str, H5LT_lang_t lang_type, size_t *len)
21892190
}
21902191

21912192
/*-------------------------------------------------------------------------
2192-
* Function: H5LT_append_dtype_super_text
2193+
* Function: append_dtype_super_text
21932194
*
21942195
* Purpose: Helper function to get super type text and append it to dt_str.
21952196
* This encapsulates the common pattern of: allocate buffer,
@@ -2200,21 +2201,23 @@ H5LTdtype_to_text(hid_t dtype, char *str, H5LT_lang_t lang_type, size_t *len)
22002201
*-------------------------------------------------------------------------
22012202
*/
22022203
static char *
2203-
H5LT_append_dtype_super_text(hid_t super, char *dt_str, H5LT_lang_t lang, size_t *slen, bool no_user_buf)
2204+
append_dtype_super_text(hid_t super, char *dt_str, H5LT_lang_t lang, size_t *slen, bool no_user_buf)
22042205
{
22052206
size_t super_len;
22062207
char *stmp = NULL;
22072208

22082209
/* Get required buffer size for super type text */
22092210
if (H5LTdtype_to_text(super, NULL, lang, &super_len) < 0)
22102211
return NULL;
2212+
/* Treat zero-length result as failure rather than calling calloc(0,...) */
22112213
if (super_len == 0)
22122214
return NULL;
22132215

22142216
/* Allocate buffer for super type text */
22152217
stmp = (char *)calloc(super_len, sizeof(char));
2216-
if (!stmp)
2217-
return NULL;
2218+
if (!stmp) {
2219+
H5Epush_ret(__func__, H5E_ERR_CLS, H5E_RESOURCE, H5E_NOSPACE, "memory allocation failed", NULL);
2220+
}
22182221

22192222
/* Convert super type to text */
22202223
if (H5LTdtype_to_text(super, stmp, lang, &super_len) < 0) {
@@ -2634,7 +2637,7 @@ H5LT_dtype_to_text(hid_t dtype, char *dt_str, H5LT_lang_t lang, size_t *slen, bo
26342637
if ((super = H5Tget_super(dtype)) < 0)
26352638
goto out;
26362639
{
2637-
char *tmp = H5LT_append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
2640+
char *tmp = append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
26382641
H5Tclose(super);
26392642
if (!tmp) {
26402643
dt_str = NULL; /* freed by callee's realloc_and_append on failure */
@@ -2673,7 +2676,7 @@ H5LT_dtype_to_text(hid_t dtype, char *dt_str, H5LT_lang_t lang, size_t *slen, bo
26732676
if ((super = H5Tget_super(dtype)) < 0)
26742677
goto out;
26752678
{
2676-
char *tmp = H5LT_append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
2679+
char *tmp = append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
26772680
H5Tclose(super);
26782681
if (!tmp) {
26792682
dt_str = NULL; /* freed by callee's realloc_and_append on failure */
@@ -2727,7 +2730,7 @@ H5LT_dtype_to_text(hid_t dtype, char *dt_str, H5LT_lang_t lang, size_t *slen, bo
27272730
if ((super = H5Tget_super(dtype)) < 0)
27282731
goto out;
27292732
{
2730-
char *tmp = H5LT_append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
2733+
char *tmp = append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
27312734
H5Tclose(super);
27322735
if (!tmp) {
27332736
dt_str = NULL; /* freed by callee's realloc_and_append on failure */
@@ -2832,7 +2835,7 @@ H5LT_dtype_to_text(hid_t dtype, char *dt_str, H5LT_lang_t lang, size_t *slen, bo
28322835
if ((super = H5Tget_super(dtype)) < 0)
28332836
goto out;
28342837
{
2835-
char *tmp = H5LT_append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
2838+
char *tmp = append_dtype_super_text(super, dt_str, lang, slen, no_user_buf);
28362839
H5Tclose(super);
28372840
if (!tmp) {
28382841
dt_str = NULL; /* freed by callee's realloc_and_append on failure */

hl/src/H5TB.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3038,8 +3038,9 @@ H5TBget_field_info(hid_t loc_id, const char *dset_name, char *field_names[], siz
30383038
field_names[i][HLTB_MAX_FIELD_LEN - 1] = '\0';
30393039
}
30403040
else {
3041-
/* name_len < HLTB_MAX_FIELD_LEN, so name_len + 1 <= HLTB_MAX_FIELD_LEN.
3042-
* Callers must provide buffers of at least HLTB_MAX_FIELD_LEN bytes each
3041+
/* name fits within the limit: copy only name_len + 1 bytes (more efficient
3042+
* than copying HLTB_MAX_FIELD_LEN - 1 bytes). name_len + 1 <= HLTB_MAX_FIELD_LEN,
3043+
* and callers must provide buffers of at least HLTB_MAX_FIELD_LEN bytes each
30433044
* (documented in H5TBpublic.h), so this copy is within bounds. */
30443045
memcpy(field_names[i], member_name, name_len + 1);
30453046
}

0 commit comments

Comments
 (0)