[PATCH 0/3] Enable two testcases in kselftest

[PATCH 0/3] Enable two testcases in kselftest

[Bamvor Jian Zhang]

Capabilities and ipc exist in selftest but is not enabled in the top
level Makefile. I do not know whether it is intended to.

It seems that both of them are tiny tests. These patches try to enable
them and make them build successful in cross compiling environment.

Bamvor Jian Zhang (3):
  selftests/capabilities: align the usage of CFLAGS with others
  selftests/capabilities: actually test it
  selftest/ipc: actually test it

 tools/testing/selftests/Makefile              | 2 ++
 tools/testing/selftests/capabilities/Makefile | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

-- 
2.1.4

[PATCH 1/3] selftests/capabilities: align the usage of CFLAGS with others

[Bamvor Jian Zhang]

User need to provide cap-ng.h and libcap-ng.so for cross compiling
which could be done by pass extra cflags to EXTRA_CLAGS. But other
testcases pass CFLAGS instead.

Change CLAGS ':=' to CLAGS '+=' to align with others. Delete useless
EXTRA_CLAGS at the same time.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 tools/testing/selftests/capabilities/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/capabilities/Makefile b/tools/testing/selftests/capabilities/Makefile
index 8c8f0c1..47337fb 100644
--- a/tools/testing/selftests/capabilities/Makefile
+++ b/tools/testing/selftests/capabilities/Makefile
@@ -7,7 +7,7 @@ include ../lib.mk
 TARGETS := validate_cap test_execve
 TEST_PROGS := test_execve
 
-CFLAGS := -O2 -g -std=gnu99 -Wall -lcap-ng
+CFLAGS += -O2 -g -std=gnu99 -Wall -lcap-ng
 
 all: $(TARGETS)
 
@@ -15,4 +15,4 @@ clean:
 	$(RM) $(TARGETS)
 
 $(TARGETS): %: %.c
-	$(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
+	$(CC) -o $@ $(CFLAGS) $^ -lrt -ldl
-- 
2.1.4

Re: [PATCH 1/3] selftests/capabilities: align the usage of CFLAGS with others

[Michael Ellerman]

On Sat, 2015-11-14 at 14:01 +0800, Bamvor Jian Zhang wrote:

> User need to provide cap-ng.h and libcap-ng.so for cross compiling
> which could be done by pass extra cflags to EXTRA_CLAGS. But other
> testcases pass CFLAGS instead.
> 
> Change CLAGS ':=' to CLAGS '+=' to align with others. Delete useless
> EXTRA_CLAGS at the same time.

Actually I think you can clean this up even more. I don't see any reason it
can't use the implicit rule for compilation.

Also libraries should be in LDLIBS, not CFLAGS. And we can change the ordering
so that we don't need to define all multiple times.

Can you test this works for you?

cheers

diff --git a/tools/testing/selftests/capabilities/Makefile b/tools/testing/selftests/capabilities/Makefile
index 8c8f0c1f0889..5f690c1ae894 100644
--- a/tools/testing/selftests/capabilities/Makefile
+++ b/tools/testing/selftests/capabilities/Makefile
@@ -1,18 +1,14 @@
-all:
-
-include ../lib.mk
-
-.PHONY: all clean
-
 TARGETS := validate_cap test_execve
 TEST_PROGS := test_execve
 
-CFLAGS := -O2 -g -std=gnu99 -Wall -lcap-ng
+CFLAGS += -O2 -g -std=gnu99 -Wall
+LDLIBS += -lcap-ng -lrt -ldl
 
 all: $(TARGETS)
 
+include ../lib.mk
+
 clean:
 	$(RM) $(TARGETS)
 
-$(TARGETS): %: %.c
-	$(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
+.PHONY: all clean

Re: [PATCH 1/3] selftests/capabilities: align the usage of CFLAGS with others

[Bamvor Zhang Jian]

Hi, Michael

On 11/16/2015 01:16 PM, Michael Ellerman wrote:
> On Sat, 2015-11-14 at 14:01 +0800, Bamvor Jian Zhang wrote:
>
>> User need to provide cap-ng.h and libcap-ng.so for cross compiling
>> which could be done by pass extra cflags to EXTRA_CLAGS. But other
>> testcases pass CFLAGS instead.
>>
>> Change CLAGS ':=' to CLAGS '+=' to align with others. Delete useless
>> EXTRA_CLAGS at the same time.
>
> Actually I think you can clean this up even more. I don't see any reason it
> can't use the implicit rule for compilation.
>
> Also libraries should be in LDLIBS, not CFLAGS. And we can change the ordering
> so that we don't need to define all multiple times.
>
> Can you test this works for you?
>
> cheers
>
> diff --git a/tools/testing/selftests/capabilities/Makefile b/tools/testing/selftests/capabilities/Makefile
> index 8c8f0c1f0889..5f690c1ae894 100644
> --- a/tools/testing/selftests/capabilities/Makefile
> +++ b/tools/testing/selftests/capabilities/Makefile
> @@ -1,18 +1,14 @@
> -all:
> -
> -include ../lib.mk
> -
> -.PHONY: all clean
> -
>  TARGETS := validate_cap test_execve
>  TEST_PROGS := test_execve
>
> -CFLAGS := -O2 -g -std=gnu99 -Wall -lcap-ng
> +CFLAGS += -O2 -g -std=gnu99 -Wall
> +LDLIBS += -lcap-ng -lrt -ldl
>
>  all: $(TARGETS)
>
> +include ../lib.mk
> +
>  clean:
>   $(RM) $(TARGETS)
>
> -$(TARGETS): %: %.c
> - $(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
> +.PHONY: all clean
>
Yes, and with your comment in 2/3, how about this one:
diff --git a/tools/testing/selftests/capabilities/Makefile
b/tools/testing/selftests/capabilities/Makefile
index 8c8f0c1..de07767 100644
--- a/tools/testing/selftests/capabilities/Makefile
+++ b/tools/testing/selftests/capabilities/Makefile
@@ -1,18 +1,13 @@
-all:
-
-include ../lib.mk
-
-.PHONY: all clean
-
-TARGETS := validate_cap test_execve
+TEST_FILES := validate_cap
 TEST_PROGS := test_execve

-CFLAGS := -O2 -g -std=gnu99 -Wall -lcap-ng
+CFLAGS += -O2 -g -std=gnu99 -Wall
+LDLIBS += -lcap-ng -lrt -ldl

-all: $(TARGETS)
+all: ${TEST_PROGS} ${TEST_FILES}

 clean:
- $(RM) $(TARGETS)
+ ${RM} ${TEST_PROGS} ${TEST_FILES}
+
+include ../lib.mk

-$(TARGETS): %: %.c
- $(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl

Re: [PATCH 1/3] selftests/capabilities: align the usage of CFLAGS with others

[Michael Ellerman]

On Mon, 2015-11-16 at 16:52 +0800, Bamvor Zhang Jian wrote:
> Hi, Michael
> On 11/16/2015 01:16 PM, Michael Ellerman wrote:
> > On Sat, 2015-11-14 at 14:01 +0800, Bamvor Jian Zhang wrote:
> > > User need to provide cap-ng.h and libcap-ng.so for cross compiling
> > > which could be done by pass extra cflags to EXTRA_CLAGS. But other
> > > testcases pass CFLAGS instead.
> > > 
> > > Change CLAGS ':=' to CLAGS '+=' to align with others. Delete useless
> > > EXTRA_CLAGS at the same time.
> > 
> > Actually I think you can clean this up even more. I don't see any reason it
> > can't use the implicit rule for compilation.

> Yes, and with your comment in 2/3, how about this one:

Yeah that's fine. You have to say ${TEST_FILES} ${TEST_PROGS} twice, but that's
not so bad.

I think it's more common to use $( ) brackets in Makefiles, so I'd prefer you
used that rather than ${ }.

cheers

> diff --git a/tools/testing/selftests/capabilities/Makefile
> b/tools/testing/selftests/capabilities/Makefile
> index 8c8f0c1..de07767 100644
> --- a/tools/testing/selftests/capabilities/Makefile
> +++ b/tools/testing/selftests/capabilities/Makefile
> @@ -1,18 +1,13 @@
> -all:
> -
> -include ../lib.mk
> -
> -.PHONY: all clean
> -
> -TARGETS := validate_cap test_execve
> +TEST_FILES := validate_cap
>  TEST_PROGS := test_execve
> 
> -CFLAGS := -O2 -g -std=gnu99 -Wall -lcap-ng
> +CFLAGS += -O2 -g -std=gnu99 -Wall
> +LDLIBS += -lcap-ng -lrt -ldl
> 
> -all: $(TARGETS)
> +all: ${TEST_PROGS} ${TEST_FILES}
> 
>  clean:
> - $(RM) $(TARGETS)
> + ${RM} ${TEST_PROGS} ${TEST_FILES}
> +
> +include ../lib.mk
> 
> -$(TARGETS): %: %.c
> - $(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 2/3] selftests/capabilities: actually test it

[Bamvor Jian Zhang]

The capatabilities exist in selftest but no in the TARGET list.
Add it to the TARGET.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 tools/testing/selftests/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 2458288..71b147e 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -1,4 +1,5 @@
 TARGETS = breakpoints
+TARGETS += capabilities
 TARGETS += cpu-hotplug
 TARGETS += efivarfs
 TARGETS += exec
-- 
2.1.4

Re: [PATCH 2/3] selftests/capabilities: actually test it

[Michael Ellerman]

On Sat, 2015-11-14 at 14:01 +0800, Bamvor Jian Zhang wrote:

> The capatabilities exist in selftest but no in the TARGET list.
> Add it to the TARGET.
> 
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  tools/testing/selftests/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 2458288..71b147e 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -1,4 +1,5 @@
>  TARGETS = breakpoints
> +TARGETS += capabilities
>  TARGETS += cpu-hotplug
>  TARGETS += efivarfs
>  TARGETS += exec


This breaks when you use TARGETS on the command line, eg:

 $ make -C tools/testing/selftests TARGETS=capabilities
 make[1]: *** No rule to make target 'capabilities', needed by 'all'.  Stop.

That's because the capabilities/Makefile uses the variable TARGETS internally.
You need to change the name of that variable in capabilities/Makefile, eg. to
BINARIES or something.

cheers

Re: [PATCH 2/3] selftests/capabilities: actually test it

[Andy Lutomirski]

On Sun, Nov 15, 2015 at 9:18 PM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
> On Sat, 2015-11-14 at 14:01 +0800, Bamvor Jian Zhang wrote:
>
>> The capatabilities exist in selftest but no in the TARGET list.
>> Add it to the TARGET.
>>
>> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  tools/testing/selftests/Makefile | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index 2458288..71b147e 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -1,4 +1,5 @@
>>  TARGETS = breakpoints
>> +TARGETS += capabilities
>>  TARGETS += cpu-hotplug
>>  TARGETS += efivarfs
>>  TARGETS += exec
>
>
> This breaks when you use TARGETS on the command line, eg:

Taking a step back for a second: isn't the variable you want
TEST_PROGS, not TARGETS?

--Andy

Re: [PATCH 2/3] selftests/capabilities: actually test it

[Andy Lutomirski]

On Mon, Nov 16, 2015 at 10:49 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Sun, Nov 15, 2015 at 9:18 PM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
>> On Sat, 2015-11-14 at 14:01 +0800, Bamvor Jian Zhang wrote:
>>
>>> The capatabilities exist in selftest but no in the TARGET list.
>>> Add it to the TARGET.
>>>
>>> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>>  tools/testing/selftests/Makefile | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>>> index 2458288..71b147e 100644
>>> --- a/tools/testing/selftests/Makefile
>>> +++ b/tools/testing/selftests/Makefile
>>> @@ -1,4 +1,5 @@
>>>  TARGETS = breakpoints
>>> +TARGETS += capabilities
>>>  TARGETS += cpu-hotplug
>>>  TARGETS += efivarfs
>>>  TARGETS += exec
>>
>>
>> This breaks when you use TARGETS on the command line, eg:
>
> Taking a step back for a second: isn't the variable you want
> TEST_PROGS, not TARGETS?

Never mind, I read the patch wrong.

--Andy

[PATCH 3/3] selftest/ipc: actually test it

[Bamvor Jian Zhang]

The ipc testcase exist in selftest but no in the TARGET list.
Add it to the TARGET.

Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 tools/testing/selftests/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 71b147e..26dff5f 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -6,6 +6,7 @@ TARGETS += exec
 TARGETS += firmware
 TARGETS += ftrace
 TARGETS += futex
+TARGETS += ipc
 TARGETS += kcmp
 TARGETS += membarrier
 TARGETS += memfd
-- 
2.1.4