On 10/15/24 03:00, zhouyuhang wrote:


[snip] for clarity.

Why is this necessary? This is defined in linux/capability.h.

Sorry for not noticing this before.
This is to be compatible with some older versions of linux/capability.h that do 
not define this macro.


+int capget(cap_user_header_t header, cap_user_data_t data);
+int capset(cap_user_header_t header, const cap_user_data_t data);

In general prototypes such as these should be defined in header
file. Why are we defining these here?

These are defined in sys/capability.h

I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.


I tested it on my machine without libcap-dev installed, the 
/usr/include/linux/capability.h

is on this machine by default. Successfully compiled using #include 
<linux/capability.h>

but not with #include <sys/capability.h>. This patch removes libcap library 
dependencies.

And we don't use any part of sys/capability.h other than these two syscalls. So 
I think that's why it's necessary.

You are changing the code to not include sys/capability.h
What happens if sys/capability.h along with linux/capability.h

Do you see problems?




I'm sorry, maybe I wasn't clear enough.
When we install the libcap library it will have the following output:

test@test:~/work/libcap$ sudo make install | grep capability
install -m 0644 include/sys/capability.h /usr/include/sys
install -m 0644 include/sys/capability.h /usr/include/sys
/usr/share/man/man5 capability.conf.5 \

It installs sys/capability.h in the correct location, but does not

install linux/capability.h, so sys/capability.h is bound to the libcap library

It won't install inux/capability.h unless you run "make headers" in
the kernel repo.


and they will either exist or disappear together. Now I want to remove

the dependency of the test on libcap library so I changed the code that it

does not contain sys/capability.h but instead linux/capability.h,

so that the test can compile successfully without libcap being installed,

these two syscalls are not declared in linux/capability.h(It is sufficient for 
test use except for these two syscalls)

so we need to declare them here. I think that's why the commit 663af70aabb7

("bpf: selftests: Add helpers to directly use the capget and capset syscall") I 
refered to

does the same thing. As for your question "What happens if sys/capability.h 
along

with linux/capability.h", I haven't found the problem yet, I sincerely hope you 
can

help me improve this code. Thank you very much.

Try this:

Run make headers in the kernel repo.
Build without making any changes.
Then add you changes and add linux/capability.h to include files

Tell me what happens.

Try the above first.


The change you are making isn't correct. Because you don't want to
define system calls locally in your source file.


Thanks, I see.
Maybe I should move them to a new header file and resend a patch.

No. Please see above. I would rather not see these defined at all
locally.

thanks,
-- Shuah

Reply via email to