by Muhammad Usama Anjum

[permalink] [raw]

Subject: Re: [PATCH v2 5/5] selftests: Drop duplicate -D_GNU_SOURCE

On 5/8/24 2:38 AM, Edward Liaw wrote:
> -D_GNU_SOURCE can be de-duplicated here, as it is added by
> KHDR_INCLUDES.
>
> Signed-off-by: Edward Liaw <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>

> ---
> tools/testing/selftests/futex/functional/Makefile | 2 +-
> tools/testing/selftests/iommu/Makefile | 2 --
> tools/testing/selftests/net/tcp_ao/Makefile | 2 +-
> tools/testing/selftests/resctrl/Makefile | 2 +-
> 4 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
> index a392d0917b4e..f79f9bac7918 100644
> --- a/tools/testing/selftests/futex/functional/Makefile
> +++ b/tools/testing/selftests/futex/functional/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> INCLUDES := -I../include -I../../ $(KHDR_INCLUDES)
> -CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES) $(KHDR_INCLUDES)
> +CFLAGS := $(CFLAGS) -g -O2 -Wall -pthread $(INCLUDES) $(KHDR_INCLUDES)
> LDLIBS := -lpthread -lrt
>
> LOCAL_HDRS := \
> diff --git a/tools/testing/selftests/iommu/Makefile b/tools/testing/selftests/iommu/Makefile
> index 32c5fdfd0eef..fd6477911f24 100644
> --- a/tools/testing/selftests/iommu/Makefile
> +++ b/tools/testing/selftests/iommu/Makefile
> @@ -2,8 +2,6 @@
> CFLAGS += -Wall -O2 -Wno-unused-function
> CFLAGS += $(KHDR_INCLUDES)
>
> -CFLAGS += -D_GNU_SOURCE
> -
> TEST_GEN_PROGS :=
> TEST_GEN_PROGS += iommufd
> TEST_GEN_PROGS += iommufd_fail_nth
> diff --git a/tools/testing/selftests/net/tcp_ao/Makefile b/tools/testing/selftests/net/tcp_ao/Makefile
> index 522d991e310e..c608b1ec02e6 100644
> --- a/tools/testing/selftests/net/tcp_ao/Makefile
> +++ b/tools/testing/selftests/net/tcp_ao/Makefile
> @@ -26,7 +26,7 @@ LIB := $(LIBDIR)/libaotst.a
> LDLIBS += $(LIB) -pthread
> LIBDEPS := lib/aolib.h Makefile
>
> -CFLAGS := -Wall -O2 -g -D_GNU_SOURCE -fno-strict-aliasing
> +CFLAGS := -Wall -O2 -g -fno-strict-aliasing
> CFLAGS += $(KHDR_INCLUDES)
> CFLAGS += -iquote ./lib/ -I ../../../../include/
>
> diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
> index 2deac2031de9..5073dbc96125 100644
> --- a/tools/testing/selftests/resctrl/Makefile
> +++ b/tools/testing/selftests/resctrl/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE
> +CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2
> CFLAGS += $(KHDR_INCLUDES)
>
> TEST_GEN_PROGS := resctrl_tests

--
BR,
Muhammad Usama Anjum

2024-05-08 12:06:26

On 5/7/24 15:38, Edward Liaw wrote:
> 809216233555 ("selftests/harness: > asprintf into kselftest_harness.h, which > _GNU_SOURCE to either be defined prior > -D_GNU_SOURCE flag passed to the compiler.
> v1: https://lore.kernel.org/linux-kselftest/[email protected]/
so that it is in a single
from source code to resolve
with -D_GNU_SOURCE
in Makefile
to be split up.
Makefile
/> Otherwise it will
resolve.
| 4 ++--
| 2 +-
| 2 +-
| 2 --
| 2 --
| 2 --
point_restore.c | 2 --
| 2 --
| 1 -
| 2 --
| 2 --
| 1 -
| 2 +-
| 2 --
binderfs_test.c | 2 --
| 1 -
| 1 -
ll_wakeup_test.c | 2 --
| 2 --
| 2 --
| 2 +-
/dev_in_maps.c | 2 --
tatmount_test.c | 3 ---
| 2 +-
x_requeue_pi.c | 3 ---
| 2 +-
| 2 --
| 1 -
| 2 --
| 2 +-
| 2 --
| 1 -
| 3 ---
| 1 -
| 3 ---
| 3 ---
| 3 ---
| 2 --
| 3 ---
| 3 ---
| 2 --
| 2 --
| 3 ---
| 3 ---
| 2 --
| 2 --
| 2 --
| 3 ---
| 1 -
| 2 --
n_stress_test.c | 3 ---
| 3 ---
| 1 -
| 2 --
| 2 --
| 1 -
| 1 -
| 2 --
| 3 ---
| 2 --
| 2 --
| 1 -
| 2 --
| 1 -
| 2 --
eptions_test.c | 2 --
| 3 ---
| 2 --
| 2 --
| 3 ---
| 1 -
| 1 -
| 1 -
mulation_test.c | 3 ---
| 1 -
| 1 -
| 2 --
| 2 --
msr_exit_test.c | 2 --
| 3 ---
| 1 -
on_timer_test.c | 1 -
| 2 --
| 1 -
| 2 --
| 2 --
| 2 --
| 2 --
| 2 --
| 2 +-
| 2 --
| 2 --
| 2 --
| 2 --
| 1 -
| 1 -
| 1 -
| 1 -
| 2 --
| 1 -
| 1 -
| 1 -
| 1 -
| 2 --
| 2 --
| 2 --
| 1 -
| 1 -
| 1 -
| 2 --
| 1 -
| 2 --
| 1 -
| 1 -
| 1 -
| 2 --
| 1 -
| 1 -
| 1 -
| 2 --
| 2 --
| 1 -
| 1 -
| 1 -
| 2 +-
| 2 --
| 1 -
| 2 --
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 2 --
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 1 -
| 1 -
| 3 ---
| 3 ---
| 2 +-
| 1 -
| 2 --
| 1 -
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 2 --
| 2 --
| 1 -
| 1 -
| 2 --
| 2 --
| 2 --
| 2 --
| 2 --
| 2 --
| 2 --
| 2 ++
| 2 +-
| 1 -
| 1 -
| 2 +-
| 2 +-
| 1 -
| 2 --
| 1 -
| 2 --
| 2 +-
| 1 -
| 2 --
| 2 +-
| 1 -
| 2 +-
| 2 --
| 2 --
| 3 ---
| 3 ---
| 3 ---
| 2 --
| 2 --
| 2 --
| 2 --
| 1 -
| 2 --
| 2 --
| 2 --
| 2 --
| 2 --
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 3 ---
| 4 ----
| 4 ----
| 4 ----
| 1 -
| 3 ---
| 4 ----
| 3 ---
| 3 ---
| 3 ---
426 deletions(-)

2024-05-09 05:15:34

On 5/9/24 10:45 PM, Edward Liaw wrote:
> On Thu, May 9, 2024 at 7:37 AM Shuah Khan <[email protected]> wrote:
>>
>> On 5/9/24 00:13, Edward Liaw wrote:
>>> On Wed, May 8, 2024 at 4:10 PM Shuah Khan <[email protected]> wrote:
>>>>
>>>> On 5/7/24 15:38, Edward Liaw wrote:
>>>>> 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced
>>>>> asprintf into kselftest_harness.h, which is a GNU extension and needs
>>>>> _GNU_SOURCE to either be defined prior to including headers or with the
>>>>> -D_GNU_SOURCE flag passed to the compiler.
>>>>>
>>>>> v1: https://lore.kernel.org/linux-kselftest/[email protected]/
>>>>> v2: add -D_GNU_SOURCE to KHDR_INCLUDES so that it is in a single
>>>>> location. Remove #define _GNU_SOURCE from source code to resolve
>>>>> redefinition warnings.
>>>>>
>>>>> Edward Liaw (5):
>>>>> selftests: Compile kselftest headers with -D_GNU_SOURCE
>>>>> selftests/sgx: Include KHDR_INCLUDES in Makefile
>>>>
>>>> I appled patches 1/5 and 2.5 - The rest need to be split up.
>>>>
>>>>> selftests: Include KHDR_INCLUDES in Makefile
>>>>> selftests: Drop define _GNU_SOURCE
>>>>> selftests: Drop duplicate -D_GNU_SOURCE
>>>>>
>>>>
>>>> Please split these patches pwe test directory. Otherwise it will
>>>> cause merge conflicts which can be hard to resolve.
>>>
>>> Hi Shuah,
>>> Sean asked that I rebase the patches on linux-next, and I will need to
>>> remove additional _GNU_SOURCE defines. Should I send an unsplit v3 to
>>> be reviewed, then split it afterwards? I'm concerned that it will be
>>> difficult to review with ~70 patches once split.
>>
>> Please send them split - it will be easier to review and apply. You
>> might as well wait until the merge window is done. I don't think
>> anybody would have time to review now since merge window starts
>> next week.
>
> Sorry, I have them split already; is it ok if I send them now? I will
> be on leave soon and may not be able to get back to it in a while.
Feel free to send the patches. There is no restriction on that.

>
> Thanks,
> Edward
>
>>
>>
>> thanks,
>> -- Shuah
>

--
BR,
Muhammad Usama Anjum