Re: [PATCH] box/memtx: strip_core -- Warn on linux only

[Alexander Turenko <alexander.turenko@tarantool.org>]

LGTM.

My thoughts on the topic are below.

WBR, Alexander Turenko.

On Wed, Aug 28, 2019 at 09:13:22PM +0300, Cyrill Gorcunov wrote:
> We know that madvise(MADV_DONTDUMP) is present
> on linux based platforms only (macos doesn't
> support it at all, freebsd requires MADV_NOCORE
> or similar which is unsupported by now) thus
> we should not print a warning on other systems
> to not spam users.
> 
> Fixes #4464
> ---
> Sasha, could you please check if it fixes the problem?

The warning is not shown on Mac OS at box.cfg{} after the patch.

> 
>  src/main.cc | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/main.cc b/src/main.cc
> index 5776aa41d..b16cfa5fe 100644
> --- a/src/main.cc
> +++ b/src/main.cc
> @@ -496,7 +496,19 @@ load_cfg()
>  	 */
>  	if (cfg_geti("strip_core")) {
>  		if (!small_test_feature(SMALL_FEATURE_DONTDUMP)) {
> -			say_warn("'strip_core' is set but unsupported");
> +			static const char strip_msg[] =
> +				"'strip_core' is set but unsupported";
> +#ifdef TARGET_OS_LINUX
> +			/*
> +			 * Linux is known to support madvise(DONT_DUMP)
> +			 * feature, thus warn on this platform only. The
> +			 * rest should be notified on verbose level only
> +			 * to not spam a user.
> +			 */
> +			say_warn(strip_msg);
> +#else
> +			say_verbose(strip_msg);
> +#endif

I think it is okay: it is the minor thing and an easiest way to solve is
the better way.

If we'll have more OS dependent features (hopefully we'll not), then
maybe it will be worth to do two things:

* Check whether a user asks to enable something explicitly (or it was
  set by default); give a warning only for an explicit choose.
* Check whether libsmall was compiled with a feature supported and don't
  give a warning if a tarantool build does not support a feature on a
  platform at all (it is instead of TARGET_OS_LINUX check).

Those bullets implemented will give us the following logic: give a
warning only if a user explicitly asks a feature AND it is supported on
a platform AND it is not supported at runtime.