HDFGroup / HDFGroup/hdf5

H5Pget_driver_info() pushes an error when the driver has no info block, contrary to its documented contract

Open
#6,651 0 comments 0 reactions 1 assignee Claimed by @fortnern View on GitHub
Dominant language
C
Stars
988
Forks
355
Avg merge
4d 2h
Merged PRs (30d)
12

Description

### Summary

`H5Pget_driver_info()` pushes an error onto the error stack when the driver has
registered no driver-specific properties. Both its own header comment and its entry in
`H5Ppublic.h` say that case returns NULL with **no** error pushed.

`src/H5Pfapl.c`:

```c
/*
* Failure: NULL. Null is also returned if the driver has
* not registered any driver-specific properties
* although no error is pushed on the stack in
* this case.
*/
const void *
H5Pget_driver_info(hid_t plist_id)
{
...
/* Get the driver info */
if (NULL == (ret_value = (const void *)H5P_peek_driver_info(plist)))
HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, NULL, "can't get driver info");
```

`H5Ppublic.h` states the same contract to users:

> If no driver-specific properties have been registered, H5Pget_driver_info() returns NULL.

`H5P_peek_driver_info()` returns `driver_prop.driver_info`, which is legitimately NULL
whenever the driver was selected without an info block — which is what
`H5Pset_driver_by_name()` and the `HDF5_DRIVER` environment variable both do. The
function cannot distinguish that from a failure, so it reports every one as a failure.

The sibling `H5Pget_driver_config_str()` handles the same situation correctly:

```c
if ((config_str = H5P_peek_driver_config_str(plist))) {
```

### Effect

Any VFD that fetches its optional FAPL info block sees an HDF5-DIAG stack printed on
every open, describing an error that did not occur:

```
HDF5-DIAG: Error detected in HDF5 (2.3.0):
#000: src/H5Pfapl.c line 1546 in H5Pget_driver_info(): can't get driver info
major: Property lists
minor: Can't get value
```

HDF5's own `src/H5FDmulti.c` calls it in exactly this pattern, in four places, and treats
NULL as "no info block":

```c
fa = (const H5FD_multi_fapl_t *)H5Pget_driver_info(fapl_id);
```

The noise is harmless to the operation, but it is not harmless to testing: any test that
compares a program's stderr against a reference sees a diff, and anyone reading the
output of a driver under development is told there is an error when there is not.

I hit it running the netCDF-C test suite over an out-of-tree VFD selected with
`HDF5_DRIVER=`; every `H5Fopen` in the run produced one of these blocks.

### Fix

Drop the wrapper `HGOTO_ERROR` and assign the result. The two genuine failures still
report themselves — `H5P_peek_driver_info()` pushes `H5E_BADTYPE` for a list that is not
a file access property list, and `H5E_CANTGET` when the property cannot be read — so
nothing that was diagnosed stops being diagnosed; only the frame that fired on a normal
NULL goes away. That also removes the duplicate second frame stacked on top of the two
real errors.

PR to follow.

### Environment

HDF5 `develop` @ `b7b85e7abf9`, Windows 11, MSVC 19.x. The code is platform-independent
and dates to at least [svn-r15510] (2008), so this is not a recent regression.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.