Re: [Xen-devel] [PATCH for-next 6/9] kconfig: add llvm coverage option

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.10.17 at 11:19, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -28,10 +28,17 @@ config FRAME_POINTER
>         maybe slower, but it gives very useful debugging information
>         in case of any Xen bugs.
>  
> +# Hidden option enabled when either GCOV or LLVM coverage support is enabled.
> +# This allows to enable the shared coverage bits in Xen without having to
> +# check for each possible coverage implementation.
> +config COVERAGE
> +     bool

In case this is still needed after the conversion to "choice", the
comment should be dropped - it is the very purpose of
prompt-less options that is being described there.

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -119,6 +119,10 @@ ifeq ($(CONFIG_GCOV),y)
>  $(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS 
> += -fprofile-arcs -ftest-coverage
>  endif
>  
> +ifeq ($(CONFIG_LLVM_COVERAGE),y)
> +$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS 
> += -fprofile-instr-generate -fcoverage-mapping
> +endif

The use of nogcov-y here suggests that the earlier patch abstracting
away the "g" should be extended to remove that letter here, too.

> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -396,7 +396,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> u_sysctl)
>      }
>      break;
>  
> -#ifdef CONFIG_GCOV
> +#ifdef CONFIG_COVERAGE
>      case XEN_SYSCTL_cov_op:
>          ret = sysctl_cov_op(&op->u.cov_op);
>          copyback = 1;

Couldn't you better do away with the #ifdef altogether, by
introducing a suitable inline function for the coverage-disabled
case (returning -EOPNOTSUPP)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel