Re: [LTP] [PATCH ltp] make filecaps tests succeed

["Serge E. Hallyn" <serue@us.ibm.com>]

Quoting Garrett Cooper (yanegomi@gmail.com):
> On Wed, Apr 28, 2010 at 3:47 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> > Most of these are belated cleanup after the move to using /opt/ltp.
> > Also undoing an ill-advised replacement of return with tst_exit.  All
> > filecaps tests now succeed on fedora 10.
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > ---
> >  testcases/kernel/security/filecaps/filecapstest.sh |    9 ++++++---
> >  testcases/kernel/security/filecaps/print_caps.c    |    3 ++-
> >  .../kernel/security/filecaps/verify_caps_exec.c    |   19 ++++++++++++++-----
> >  3 files changed, 22 insertions(+), 9 deletions(-)
> >
> > diff --git a/testcases/kernel/security/filecaps/filecapstest.sh b/testcases/kernel/security/filecaps/filecapstest.sh
> > index 43582dc..6864de4 100755
> > --- a/testcases/kernel/security/filecaps/filecapstest.sh
> > +++ b/testcases/kernel/security/filecaps/filecapstest.sh
> > @@ -22,8 +22,11 @@
> >  echo "Running in:"
> >  #rm -f print_caps
> >  #cp $LTPROOT/testcases/bin/print_caps .
> > -mkfifo caps_fifo
> > -chmod 777 caps_fifo
> > +#FIFOFILE="$LTPROOT/testcases/bin/caps_fifo"
> > +FIFOFILE="/tmp/caps_fifo"
> 
> Why not
> 
> TMP=${TMP:=/tmp}
> $TMP/caps_fifo
> 
> etc? FWIW if you're in $TMP already, a lot of these changes aren't
> required, correct?

No objection - though then the definition of /tmp/caps_fifo below
needs to be changed as well.

> > +rm -f $FIFOFILE
> > +mkfifo $FIFOFILE
> > +chmod 777 $FIFOFILE
> >  exit_code=0
> >  echo "cap_sys_admin tests"
> >  verify_caps_exec 0
> > @@ -46,5 +49,5 @@ if [ $tmp -ne 0 ]; then
> >        exit_code=$tmp
> >  fi
> >
> > -unlink caps_fifo
> > +unlink $FIFOFILE
> >  exit $exit_code
> > diff --git a/testcases/kernel/security/filecaps/print_caps.c b/testcases/kernel/security/filecaps/print_caps.c
> > index f0e9bce..b887738 100644
> > --- a/testcases/kernel/security/filecaps/print_caps.c
> > +++ b/testcases/kernel/security/filecaps/print_caps.c
> > @@ -36,7 +36,7 @@
> >  #include <sys/capability.h>
> >  #endif
> >
> > -#define FIFOFILE "caps_fifo"
> > +#define FIFOFILE "/tmp/caps_fifo"
> >
> >  int main(int argc, char *argv[])
> >  {
> > @@ -68,4 +68,5 @@ int main(int argc, char *argv[])
> >  #else
> >        return 0;
> >  #endif
> > +       return 0;
> 
> What's the value returned for #if ..? If it's `return 0', then why not
> just remove the other two references in the preprocessor blocks?

There was no return in that case.  So really we can just get rid of
the #else altogether.

> >  }
> > diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > index 5250007..7360d4a 100644
> > --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> > +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > @@ -43,7 +43,7 @@
> >  #include <sys/prctl.h>
> >  #include <test.h>
> >
> > -#define TSTPATH "./print_caps"
> > +#define TSTPATH "print_caps"
> 
> Ok.
> 
> >  char *TCID = "filecaps";
> >  int TST_TOTAL=1;
> >
> > @@ -70,7 +70,7 @@ void print_my_caps()
> >        cap_free(txt);
> >  }
> >
> > -int drop_root(int keep_perms)
> > +void drop_root(int keep_perms)
> >  {
> >        int ret;
> >
> > @@ -84,10 +84,19 @@ int drop_root(int keep_perms)
> >        }
> >        if (keep_perms) {
> >                cap_t cap = cap_from_text("=eip");
> > -               cap_set_proc(cap);
> > +               int ret;
> > +               if (!cap) {
> > +                       tst_resm(TBROK, "cap_from_text failed\n");
> > +                       tst_exit();
> > +               }
> > +               ret = cap_set_proc(cap);
> > +               if (ret < 0) {
> > +                       perror(cap_set_proc);
> 
> Why??? This could potentially fubar errno too...

The perror doesn't need to be there.

> > +                       tst_resm(TBROK | TERRNO, "cap_set_proc failed\n");
> > +                       tst_exit();
> 
> tst_brkm(TBROK | TERRNO, tst_exit, "cap_set..."); is better
> 
> > +               }
> >                cap_free(cap);
> >        }
> > -       tst_exit();
> >  }
> >
> >  int perms_test(void)
> > @@ -114,7 +123,7 @@ int perms_test(void)
> >        return ret;
> >  }
> >
> > -#define FIFOFILE "caps_fifo"
> > +#define FIFOFILE "/tmp/caps_fifo"
> >  void create_fifo(void)
> >  {
> >        int ret;
> > --
> > 1.6.0.6

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list