On Mon, Apr 13, 2020 at 4:18 PM William Roberts
<bill.c.roberts@xxxxxxxxx> wrote:
>
> On Sun, Apr 12, 2020 at 3:12 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
> >
> > The Python bindings for libselinux expose functions such as
> > avc_has_perm(), get_ordered_context_list(), etc. When these functions
> > encounter an error, they set errno accordingly and return a negative
> > value. In order to get the value of errno from Python code, it needs to
> > be "forwarded" in a way. This is achieved by glue code in
> > selinuxswig_python_exception.i, which implement raising an OSError
> > exception from the value of errno.
> >
> > selinuxswig_python_exception.i was only generating glue code from
> > functions declared in selinux.h and not in other headers. Add other
> > headers.
> >
> > selinuxswig_python_exception.i is generated by "bash exception.sh". Mark
> > the fact that exception.sh is a Bash script by adding a shebang. This
> > makes "shellcheck" not warn about the Bash array which is used to list
> > header files.
> >
> > Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
> > ---
> > libselinux/src/exception.sh | 18 +-
> > libselinux/src/selinuxswig_python_exception.i | 396 ++++++++++++++++++
> > 2 files changed, 412 insertions(+), 2 deletions(-)
> >
> > diff --git a/libselinux/src/exception.sh b/libselinux/src/exception.sh
> > index 33ceef804af5..644c7a05ec54 100755
> > --- a/libselinux/src/exception.sh
> > +++ b/libselinux/src/exception.sh
> > @@ -1,3 +1,5 @@
> > +#!/bin/bash
> > +
> > function except() {
> > case $1 in
> > selinux_file_context_cmp) # ignore
> > @@ -15,10 +17,22 @@ echo "
> > ;;
> > esac
> > }
> > -if ! ${CC:-gcc} -x c -c -I../include -o temp.o - -aux-info temp.aux < ../include/selinux/selinux.h
> > +
> > +# Make sure that selinux.h is included first in order not to depend on the order
> > +# in which "#include <selinux/selinux.h>" appears in other files.
> > +FILE_LIST=(
> > + ../include/selinux/selinux.h
> > + ../include/selinux/avc.h
> > + ../include/selinux/context.h
> > + ../include/selinux/get_context_list.h
> > + ../include/selinux/get_default_type.h
> > + ../include/selinux/label.h
> > + ../include/selinux/restorecon.h
> > +)
> > +if ! cat "${FILE_LIST[@]}" | ${CC:-gcc} -x c -c -I../include -o temp.o - -aux-info temp.aux
> > then
> > # clang does not support -aux-info so fall back to gcc
> > - gcc -x c -c -I../include -o temp.o - -aux-info temp.aux < ../include/selinux/selinux.h
> > + cat "${FILE_LIST[@]}" | gcc -x c -c -I../include -o temp.o - -aux-info temp.aux
> > fi
> > for i in `awk '/<stdin>.*extern int/ { print $6 }' temp.aux`; do except $i ; done
> > rm -f -- temp.aux temp.o
> > diff --git a/libselinux/src/selinuxswig_python_exception.i b/libselinux/src/selinuxswig_python_exception.i
> > index cf6582595ee7..9f1f86a5564d 100644
> > --- a/libselinux/src/selinuxswig_python_exception.i
> > +++ b/libselinux/src/selinuxswig_python_exception.i
> > @@ -952,3 +952,399 @@
> > }
> > }
> >
> > +
> > +%exception avc_sid_to_context {
> > + $action
> > + if (result < 0) {
> > + PyErr_SetFromErrno(PyExc_OSError);
> > + SWIG_fail;
> > + }
> > +}
> > +
> > +
> > +%exception avc_sid_to_context_raw {
> > + $action
> > + if (result < 0) {
> > + PyErr_SetFromErrno(PyExc_OSError);
> > + SWIG_fail;
> > + }
> > +}
[...]
>
> A few comments:
> - overall looks fine, builds and works as expected.
> - Why the double newline space on the exception swig file? The other
> .i files seem to do a single newline?
> is their something I am missing with syntax?
> - I have the following whitespace warning:
> Applying: libselinux: add missing glue code to grab errno in Python bindings
> .git/rebase-apply/patch:444: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
The last two points are due to the way the file is generated, by exception.sh:
echo "
%exception $1 {
\$action
if (result < 0) {
PyErr_SetFromErrno(PyExc_OSError);
SWIG_fail;
}
}
"
... this introduces blank lines both before and after each exception
blocks. We could remove the one after the block by using }" in the
shell script. I will submit a patch that does this once this patch is
merged, as this makes the file cleaner.
Thanks,
Nicolas
