Re: [PATCH v2 03/11] tools/libs/*: Rely on the default logger

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 12290 bytes --]

On Thu, Nov 08, 2018 at 05:07:57PM +0000, Ian Jackson wrote:
> Delete 11 entirely formulaic conditional calls to
>   xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> and associated logger_tofree variables, error handling, etc.
> 
> No overall functional change, although some memory allocation errors
> may no longer occur.
> 
> After this there are still several calls to
> xtl_createlogger_stdiostream in tree, but they almost all have
> non-default message level etc. so it is not obvious that they should
> be replaced.
> 
> The exception is in xc_private.c where xch->error_handler is
> initialised using a copy of the default initialisation boilerplate
> (ant there is the associated xch->error_handler_tofree).  However,
> there is also xch->dombuild_logger, and xch->dombuild_logger_tofree
> which is handled differently and so must be retained.  It seems better
> to keep the xch code internally consistent, and decoupled from the
> general default.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
> v2: New in this version of the series
> ---
>  tools/libs/call/core.c             | 10 ----------
>  tools/libs/call/private.h          |  2 +-
>  tools/libs/devicemodel/core.c      | 11 -----------
>  tools/libs/devicemodel/private.h   |  2 +-
>  tools/libs/evtchn/core.c           | 10 ----------
>  tools/libs/evtchn/private.h        |  2 +-
>  tools/libs/foreignmemory/core.c    | 10 ----------
>  tools/libs/foreignmemory/private.h |  2 +-
>  tools/libs/gnttab/gntshr_core.c    | 10 ----------
>  tools/libs/gnttab/gnttab_core.c    | 10 ----------
>  tools/libs/gnttab/private.h        |  2 +-
>  11 files changed, 5 insertions(+), 66 deletions(-)
> 
> diff --git a/tools/libs/call/core.c b/tools/libs/call/core.c
> index 57d3a33e6b..ee9be948e7 100644
> --- a/tools/libs/call/core.c
> +++ b/tools/libs/call/core.c
> @@ -54,14 +54,6 @@ xencall_handle *xencall_open(xentoollog_logger *logger, unsigned open_flags)
>      xcall->buffer_cache_misses = 0;
>      xcall->buffer_cache_toobig = 0;
>      xcall->logger = logger;
> -    xcall->logger_tofree = NULL;
> -
> -    if (!xcall->logger) {
> -        xcall->logger = xcall->logger_tofree =
> -            (xentoollog_logger*)
> -            xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> -        if (!xcall->logger) goto err;
> -    }
>  
>      rc = osdep_xencall_open(xcall);
>      if ( rc  < 0 ) goto err;
> @@ -71,7 +63,6 @@ xencall_handle *xencall_open(xentoollog_logger *logger, unsigned open_flags)
>  err:
>      xentoolcore__deregister_active_handle(&xcall->tc_ah);
>      osdep_xencall_close(xcall);
> -    xtl_logger_destroy(xcall->logger_tofree);
>      free(xcall);
>      return NULL;
>  }
> @@ -86,7 +77,6 @@ int xencall_close(xencall_handle *xcall)
>      xentoolcore__deregister_active_handle(&xcall->tc_ah);
>      rc = osdep_xencall_close(xcall);
>      buffer_release_cache(xcall);
> -    xtl_logger_destroy(xcall->logger_tofree);
>      free(xcall);
>      return rc;
>  }
> diff --git a/tools/libs/call/private.h b/tools/libs/call/private.h
> index 21f992b37e..a2d00b2b6a 100644
> --- a/tools/libs/call/private.h
> +++ b/tools/libs/call/private.h
> @@ -18,7 +18,7 @@
>  #endif
>  
>  struct xencall_handle {
> -    xentoollog_logger *logger, *logger_tofree;
> +    xentoollog_logger *logger;
>      unsigned flags;
>  
>                       /* partially     with /dev/     no /dev/      */
> diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
> index f76e3d305e..b91f6b4ee9 100644
> --- a/tools/libs/devicemodel/core.c
> +++ b/tools/libs/devicemodel/core.c
> @@ -46,15 +46,6 @@ xendevicemodel_handle *xendevicemodel_open(xentoollog_logger *logger,
>  
>      dmod->flags = open_flags;
>      dmod->logger = logger;
> -    dmod->logger_tofree = NULL;
> -
> -    if (!dmod->logger) {
> -        dmod->logger = dmod->logger_tofree =
> -            (xentoollog_logger*)
> -            xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> -        if (!dmod->logger)
> -            goto err;
> -    }
>  
>      dmod->xcall = xencall_open(dmod->logger, 0);
>      if (!dmod->xcall)
> @@ -67,7 +58,6 @@ xendevicemodel_handle *xendevicemodel_open(xentoollog_logger *logger,
>      return dmod;
>  
>  err:
> -    xtl_logger_destroy(dmod->logger_tofree);
>      xentoolcore__deregister_active_handle(&dmod->tc_ah);
>      xencall_close(dmod->xcall);
>      free(dmod);
> @@ -85,7 +75,6 @@ int xendevicemodel_close(xendevicemodel_handle *dmod)
>  
>      xentoolcore__deregister_active_handle(&dmod->tc_ah);
>      xencall_close(dmod->xcall);
> -    xtl_logger_destroy(dmod->logger_tofree);
>      free(dmod);
>      return rc;
>  }
> diff --git a/tools/libs/devicemodel/private.h b/tools/libs/devicemodel/private.h
> index c4a225f8af..edee969313 100644
> --- a/tools/libs/devicemodel/private.h
> +++ b/tools/libs/devicemodel/private.h
> @@ -10,7 +10,7 @@
>  #include <xentoolcore_internal.h>
>  
>  struct xendevicemodel_handle {
> -    xentoollog_logger *logger, *logger_tofree;
> +    xentoollog_logger *logger;
>      unsigned int flags;
>      xencall_handle *xcall;
>      int fd;
> diff --git a/tools/libs/evtchn/core.c b/tools/libs/evtchn/core.c
> index aff6ecfaa0..d1e53284e0 100644
> --- a/tools/libs/evtchn/core.c
> +++ b/tools/libs/evtchn/core.c
> @@ -37,18 +37,10 @@ xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger, unsigned open_flags)
>  
>      xce->fd = -1;
>      xce->logger = logger;
> -    xce->logger_tofree  = NULL;
>  
>      xce->tc_ah.restrict_callback = all_restrict_cb;
>      xentoolcore__register_active_handle(&xce->tc_ah);
>  
> -    if (!xce->logger) {
> -        xce->logger = xce->logger_tofree =
> -            (xentoollog_logger*)
> -            xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> -        if (!xce->logger) goto err;
> -    }
> -
>      rc = osdep_evtchn_open(xce);
>      if ( rc  < 0 ) goto err;
>  
> @@ -57,7 +49,6 @@ xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger, unsigned open_flags)
>  err:
>      xentoolcore__deregister_active_handle(&xce->tc_ah);
>      osdep_evtchn_close(xce);
> -    xtl_logger_destroy(xce->logger_tofree);
>      free(xce);
>      return NULL;
>  }
> @@ -71,7 +62,6 @@ int xenevtchn_close(xenevtchn_handle *xce)
>  
>      xentoolcore__deregister_active_handle(&xce->tc_ah);
>      rc = osdep_evtchn_close(xce);
> -    xtl_logger_destroy(xce->logger_tofree);
>      free(xce);
>      return rc;
>  }
> diff --git a/tools/libs/evtchn/private.h b/tools/libs/evtchn/private.h
> index 31e595bea2..a272895fe5 100644
> --- a/tools/libs/evtchn/private.h
> +++ b/tools/libs/evtchn/private.h
> @@ -9,7 +9,7 @@
>  #include <xen/xen.h>
>  
>  struct xenevtchn_handle {
> -    xentoollog_logger *logger, *logger_tofree;
> +    xentoollog_logger *logger;
>      int fd;
>      Xentoolcore__Active_Handle tc_ah;
>  };
> diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
> index 63f12e2450..d485dd8672 100644
> --- a/tools/libs/foreignmemory/core.c
> +++ b/tools/libs/foreignmemory/core.c
> @@ -41,18 +41,10 @@ xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger *logger,
>  
>      fmem->fd = -1;
>      fmem->logger = logger;
> -    fmem->logger_tofree = NULL;
>  
>      fmem->tc_ah.restrict_callback = all_restrict_cb;
>      xentoolcore__register_active_handle(&fmem->tc_ah);
>  
> -    if (!fmem->logger) {
> -        fmem->logger = fmem->logger_tofree =
> -            (xentoollog_logger*)
> -            xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> -        if (!fmem->logger) goto err;
> -    }
> -
>      rc = osdep_xenforeignmemory_open(fmem);
>      if ( rc  < 0 ) goto err;
>  
> @@ -61,7 +53,6 @@ xenforeignmemory_handle *xenforeignmemory_open(xentoollog_logger *logger,
>  err:
>      xentoolcore__deregister_active_handle(&fmem->tc_ah);
>      osdep_xenforeignmemory_close(fmem);
> -    xtl_logger_destroy(fmem->logger_tofree);
>      free(fmem);
>      return NULL;
>  }
> @@ -75,7 +66,6 @@ int xenforeignmemory_close(xenforeignmemory_handle *fmem)
>  
>      xentoolcore__deregister_active_handle(&fmem->tc_ah);
>      rc = osdep_xenforeignmemory_close(fmem);
> -    xtl_logger_destroy(fmem->logger_tofree);
>      free(fmem);
>      return rc;
>  }
> diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
> index 8f1bf081ed..9030de9740 100644
> --- a/tools/libs/foreignmemory/private.h
> +++ b/tools/libs/foreignmemory/private.h
> @@ -19,7 +19,7 @@
>  #endif
>  
>  struct xenforeignmemory_handle {
> -    xentoollog_logger *logger, *logger_tofree;
> +    xentoollog_logger *logger;
>      unsigned flags;
>      int fd;
>      Xentoolcore__Active_Handle tc_ah;
> diff --git a/tools/libs/gnttab/gntshr_core.c b/tools/libs/gnttab/gntshr_core.c
> index 1117e29c91..38cf364897 100644
> --- a/tools/libs/gnttab/gntshr_core.c
> +++ b/tools/libs/gnttab/gntshr_core.c
> @@ -31,14 +31,6 @@ xengntshr_handle *xengntshr_open(xentoollog_logger *logger, unsigned open_flags)
>  
>      xgs->fd = -1;
>      xgs->logger = logger;
> -    xgs->logger_tofree  = NULL;
> -
> -    if (!xgs->logger) {
> -        xgs->logger = xgs->logger_tofree =
> -            (xentoollog_logger*)
> -            xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> -        if (!xgs->logger) goto err;
> -    }
>  
>      rc = osdep_gntshr_open(xgs);
>      if ( rc  < 0 ) goto err;
> @@ -47,7 +39,6 @@ xengntshr_handle *xengntshr_open(xentoollog_logger *logger, unsigned open_flags)
>  
>  err:
>      osdep_gntshr_close(xgs);
> -    xtl_logger_destroy(xgs->logger_tofree);
>      free(xgs);
>      return NULL;
>  }
> @@ -60,7 +51,6 @@ int xengntshr_close(xengntshr_handle *xgs)
>          return 0;
>  
>      rc = osdep_gntshr_close(xgs);
> -    xtl_logger_destroy(xgs->logger_tofree);
>      free(xgs);
>      return rc;
>  }
> diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
> index 92e7228a26..a67f444245 100644
> --- a/tools/libs/gnttab/gnttab_core.c
> +++ b/tools/libs/gnttab/gnttab_core.c
> @@ -37,18 +37,10 @@ xengnttab_handle *xengnttab_open(xentoollog_logger *logger, unsigned open_flags)
>  
>      xgt->fd = -1;
>      xgt->logger = logger;
> -    xgt->logger_tofree  = NULL;
>  
>      xgt->tc_ah.restrict_callback = all_restrict_cb;
>      xentoolcore__register_active_handle(&xgt->tc_ah);
>  
> -    if (!xgt->logger) {
> -        xgt->logger = xgt->logger_tofree =
> -            (xentoollog_logger*)
> -            xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
> -        if (!xgt->logger) goto err;
> -    }
> -
>      rc = osdep_gnttab_open(xgt);
>      if ( rc  < 0 ) goto err;
>  
> @@ -57,7 +49,6 @@ xengnttab_handle *xengnttab_open(xentoollog_logger *logger, unsigned open_flags)
>  err:
>      xentoolcore__deregister_active_handle(&xgt->tc_ah);
>      osdep_gnttab_close(xgt);
> -    xtl_logger_destroy(xgt->logger_tofree);
>      free(xgt);
>      return NULL;
>  }
> @@ -71,7 +62,6 @@ int xengnttab_close(xengnttab_handle *xgt)
>  
>      xentoolcore__deregister_active_handle(&xgt->tc_ah);
>      rc = osdep_gnttab_close(xgt);
> -    xtl_logger_destroy(xgt->logger_tofree);
>      free(xgt);
>      return rc;
>  }
> diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
> index c5e23639b1..cdb155761e 100644
> --- a/tools/libs/gnttab/private.h
> +++ b/tools/libs/gnttab/private.h
> @@ -12,7 +12,7 @@
>  #define GSERROR(_l, _f...) xtl_log(_l, XTL_ERROR, errno, "gntshr", _f)
>  
>  struct xengntdev_handle {
> -    xentoollog_logger *logger, *logger_tofree;
> +    xentoollog_logger *logger;
>      int fd;
>      Xentoolcore__Active_Handle tc_ah;
>  };

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel