Re: [LTP] [PATCH 1/3] lapi/fcntl.h: Fix O_DIRECT definition for various archs

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> O_DIRECT definitions differ a lot depending on architecture.
> Because this can lead to problems (e.g. the generic value O_DIRECT
> 040000 is O_DIRECTORY on powerpc, address that in lapi file.
> 
> NOTE: Deliberately use correct definitions also on less common archs
> (maybe LTP even cannot be compiled on m68k or mips, but better to be
> safe then sorry).  But the problem would IMHO be avoided if the fallback
> O_DIRECT definition would be removed everywhere and tests just define
> _GNU_SOURCE (no library code is using O_DIRECT atm).
> 
> Reported-by: NeilBrown <neilb@suse.de>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  include/lapi/fcntl.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
> index 848ac7865..27da9f076 100644
> --- a/include/lapi/fcntl.h
> +++ b/include/lapi/fcntl.h
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2014 Cyril Hrubis <chrubis@suse.cz>
> + * Copyright (c) Linux Test Project, 2014-2023
>   */
>  
>  #ifndef LAPI_FCNTL_H__
> @@ -11,7 +12,15 @@
>  #include <sys/socket.h>
>  
>  #ifndef O_DIRECT
> -# define O_DIRECT 040000
> +# if defined(__mips__)
> +#  define O_DIRECT 0100000
> +# elif defined(__arm__) || defined(__aarch64__) || defined(__m68k__)
> +#  define O_DIRECT 0200000
> +# elif defined(__powerpc__) || defined(__powerpc64__)
> +#  define O_DIRECT 0400000
> +# else
> +#  define O_DIRECT 040000
> +# endif
>  #endif

I do not think that we should default to any value here, just do

#error Define O_DIRECT for your architecture

However I think that actually removing the O_DIRECT fallback and adding
_GNU_SOURCE to tests that use O_DIRECT is more future proof solution.
There is about 70 tests that use O_DIRECT and many of them define
_GNU_SOURCE already.

Actually I tried to remove the fallback from lapi/fcntl.h and recompiled
LTP without any change, which is strange. Does that work for you as
well?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp