[Patches] [ python-Patches-1615158 ] POSIX capabilities support
SourceForge.net
noreply at sourceforge.net
Sat Feb 10 20:56:45 CET 2007
Patches item #1615158, was opened at 2006-12-13 19:10
Message generated for change (Comment added) made by loewis
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1615158&group_id=5470
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Modules
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Matt Kern (gj0aqzda)
Assigned to: Nobody/Anonymous (nobody)
Summary: POSIX capabilities support
Initial Comment:
Attached is a patch which adds POSIX capabilities support. The following API functions are supported:
* cap_clear
* cap_copy_ext
* cap_dup
* cap_from_text
* cap_get_flag
* cap_get_proc
* cap_init
* cap_set_flag
* cap_set_proc
* cap_size
* cap_to_text
The following API function is supported, but is broken with certain versions of libcap (I am running debian testing's libcap1, version 1.10-14, which has an issue; I have reported this upstream):
* cap_copy_int
The following API functions are in there as stubs, but currently are not compiled. I need access to a machine to test these. I will probably add autoconf tests for availability of these functions in due course:
* cap_get_fd
* cap_get_file
* cap_set_fd
* cap_set_file
The patch includes diffs to configure. My autoconf is however at a different revision to that used on the python trunk. You may want to re-autoconf configure.in.
I've added a few API tests to test_posix.py.
----------------------------------------------------------------------
>Comment By: Martin v. Löwis (loewis)
Date: 2007-02-10 20:56
Message:
Logged In: YES
user_id=21627
Originator: NO
I don't mind the POSIX module getting bigger. In C, these functions are
all in a flat namespace, also. I like the view "if it's specified by POSIX,
you find it in the POSIX module" (although this specific API was rejected
for inclusion into POSIX). The functions are all very small, as the real
functionality is in the C library, or even the OS kernel.
As for the ifdefery: most of it is straight-forward: functionality is
either present or it isn't. It gets messy when it tries to use alternative
underlying APIs, e.g. for Win32 or OS/2. If the code is to be refactored,
this should be the way to go (i.e. move all Win32 and OS/2 implementations
out of the module)
As for PyMem_Malloc: I see no need to use that API; it doesn't improve the
code to do so, compared to malloc/free. All that matters it is symmetric.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2007-01-30 05:52
Message:
Logged In: YES
user_id=33168
Originator: NO
ISTM that this would be better as a separate module or an optional
submodule to posix. The posix module is already 8720 lines. I really
don't want it to get bigger, especially when you realize how much
#ifdef'ery is in there.
Some other things I noticed:
You should use PyMem_Malloc instead of a raw malloc (same deal with free).
Methods that take no arguments should use METH_NOARGS and then there's no
need to call PyArgs_ParseTuple (e.g., posix_cap_get_proc).
There definitely shouldn't be any abort()s in there, even if #ifdef'ed
out.
Is this 64-bit safe? My manpage (gentoo) says this: int
cap_set_flag(cap_t cap_p, cap_flag_t flag, int ncap, cap_value_t *caps,
cap_flag_value_t value);
I see that you are using ints. I don't know if that's correct on a 64-bit
platform. If not, you will need to modify the places that ints are used to
take longs.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2007-01-29 20:30
Message:
Logged In: YES
user_id=21627
Originator: NO
The patch cannot go in in its current form (I started applying it, but
then found that I just can't do it). It contains conditional, commented out
code. Either the code is correct, then it should be added, or it is
incorrect, in which case it should be removed entirely. There shouldn't be
any work-in-progress code in the Python repository whatsoever. This refers
to both the if 0 blocks (which I thought I can safely delete), as well as
commented-out entries in CapabilityStateMethods (for which I didn't know
what to do).
So while you are revising it, I have a few remarks:
- you can safely omit the generated configure changes from the patch - I
will regenerate them, anyway.
- please follow the alphabet in the header files in configure.in (bsdtty.h
< capabilities.h)
- please don't expose method on objects on which they aren't methods. E.g.
cap_clear is available both as a method and a module-level function; that
can't be both right (there should be one way to do it)
Following the socket API, I think offering these as methods is
reasonable
- try avoiding the extra copy in copy_ext (copying directly into the
string). If you keep malloc calls, don't return NULL without setting a
Python exception.
- use the "s" format for copy_int and from_text
- consider using booleans for [gs]et_flags
----------------------------------------------------------------------
Comment By: Matt Kern (gj0aqzda)
Date: 2007-01-29 17:57
Message:
Logged In: YES
user_id=1667774
Originator: YES
No news on these patches in a while.
To summarise, the patches are ready to go in. The issues surrounding
cap_copy_int(), cap_get_*() and cap_set_*() are pretty minor. The vast
majority of uses will be of the cap_get_proc(), cap_set_flag(),
cap_set_proc() variety.
I am not trying to hassle you; I know you don't have enough time to get
through everything. However, I'll hang fire on future development of stuff
that I, personally, am not going to use, until I know when/if these patches
are going to go in.
----------------------------------------------------------------------
Comment By: Matt Kern (gj0aqzda)
Date: 2006-12-19 11:48
Message:
Logged In: YES
user_id=1667774
Originator: YES
I've attached a documentation patch, which should be applied in addition
to the base patch.
File Added: patch-svn-doc.diff
----------------------------------------------------------------------
Comment By: Georg Brandl (gbrandl)
Date: 2006-12-16 14:25
Message:
Logged In: YES
user_id=849994
Originator: NO
(If you don't want to write LaTeX, it's enough to write the docs in
plaintext, there are a few volunteers who will convert it appropriately.)
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2006-12-16 13:28
Message:
Logged In: YES
user_id=21627
Originator: NO
Can you please provide documentation changes as well?
----------------------------------------------------------------------
Comment By: Matt Kern (gj0aqzda)
Date: 2006-12-13 19:12
Message:
Logged In: YES
user_id=1667774
Originator: YES
I should further add that I have implemented the following API calls as
methods of the new CapabilityState object in addition to the standard
functions:
* cap_clear
* cap_copy_ext
* cap_dup
* cap_get_flag
* cap_set_flag
* cap_set_proc
* cap_size
* cap_to_text
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1615158&group_id=5470
More information about the Patches
mailing list