On Wed, Jan 16, 2019 at 12:02:01PM -0600, William A Rowe Jr wrote:
> Doesn't this simply gloss over an underlying defect?
>
> [...]
> if (apr_dbm_fetch(f, key, &val) == APR_SUCCESS && val.dptr) {
> *value = apr_pstrmemdup(pool, val.dptr, val.dsize);
> }
>
> apr_dbm_close(f);
>
> return rv;
> }
>
> Shouldn't we capture and return the failure code from apr_dbm_fetch here?
The error code from the failed lookup is not an interesting result, I
assume because it can only fail one way. That the lookup failed is
reflected by returning with success *value set to NULL.
I had this change in my wc from looking at clang/coverity static
analysis results so I assume it was flagged up somewhere but can't
remember exactly. It seems like a harmless change.