Re: svn commit: r1917050 - in /apr/apr/trunk: CHANGES crypto/apr_crypto.c dbd/apr_dbd.c dbm/apr_dbm.c include/private/apu_internal.h util-misc/apu_dso.c

On 4/16/24 11:33 PM, [email protected] wrote:
> Author: minfrin
> Date: Tue Apr 16 21:33:58 2024
> New Revision: 1917050
> 
> URL: http://svn.apache.org/viewvc?rev=1917050&view=rev
> Log:
> Pass details of module loading errors back to the caller through the
> apu_err_t structure.
> 
> Modified:
>     apr/apr/trunk/CHANGES
>     apr/apr/trunk/crypto/apr_crypto.c
>     apr/apr/trunk/dbd/apr_dbd.c
>     apr/apr/trunk/dbm/apr_dbm.c
>     apr/apr/trunk/include/private/apu_internal.h
>     apr/apr/trunk/util-misc/apu_dso.c
> 

> Modified: apr/apr/trunk/util-misc/apu_dso.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/util-misc/apu_dso.c?rev=1917050&r1=1917049&r2=1917050&view=diff
> ==============================================================================
> --- apr/apr/trunk/util-misc/apu_dso.c (original)
> +++ apr/apr/trunk/util-misc/apu_dso.c Tue Apr 16 21:33:58 2024
> @@ -131,7 +131,8 @@ apr_status_t apu_dso_load(apr_dso_handle
>                            apr_dso_handle_sym_t *dsoptr,
>                            const char *module,
>                            const char *modsym,
> -                          apr_pool_t *pool)
> +                          apr_pool_t *pool,
> +                          apu_err_t *err)

Now the caller needs to allocate memory even if it is not interested in the error or if there is no error at all.
Wouldn't it be better to add an apu_err_t **err instead (which can be NULL) and in case of an error allocate an
apu_err_t from pool and fill it and return it in **err (provided it was not NULL).


>  {
>      apr_dso_handle_t *dlhandle = NULL;
>      char *pathlist;
> @@ -186,6 +187,7 @@ apr_status_t apu_dso_load(apr_dso_handle
>          apr_cpystrn(eos, module, sizeof(path) - (eos - path));
>  
>          rv = apr_dso_load(&dlhandle, path, global);
> +
>          if (dlhandleptr) {
>              *dlhandleptr = dlhandle;
>          }
> @@ -205,20 +207,27 @@ apr_status_t apu_dso_load(apr_dso_handle
>              apr_cpystrn(eos, module, sizeof(path) - (eos - path));
>  
>              rv = apr_dso_load(&dlhandle, path, global);
> +
>              if (dlhandleptr) {
>                  *dlhandleptr = dlhandle;
>              }
> +
>              if (rv == APR_SUCCESS) { /* APR_EDSOOPEN */
>                  break;
>              }
>          }
>      }
>  
> -    if (rv != APR_SUCCESS) /* APR_ESYMNOTFOUND */
> +    if (rv != APR_SUCCESS) { /* APR_ESYMNOTFOUND */
> +        err->msg = apr_pstrdup(pool, apr_dso_error(dlhandle, NULL, 0));
> +        err->reason = apr_pstrdup(pool, module);
>          return rv;
> +    }
>  
>      rv = apr_dso_sym(dsoptr, dlhandle, modsym);
>      if (rv != APR_SUCCESS) { /* APR_ESYMNOTFOUND */
> +        err->msg = apr_pstrdup(pool, apr_dso_error(dlhandle, NULL, 0));
> +        err->reason = apr_pstrdup(pool, module);
>          apr_dso_unload(dlhandle);
>      }
>      else {
> @@ -231,5 +240,5 @@ apr_status_t apu_dso_load(apr_dso_handle
>      return rv;
>  }
>  
> -#endif /* APR_DSO_BUILD */
> +#endif /* APR_HAVE_MODULAR_DSO */

Regards

Rüdiger