Re: [LTP] [PATCH] containers: mqns: use libc's mq_open, not syscall(__NR_mq_open

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

Quoting Garrett Cooper (yanegomi@gmail.com):
> On Tue, Dec 22, 2009 at 9:11 AM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> > The glibc version removes the leading '/' from the message queue name.
> > Not doing so makes the system call fail.  We could just remove the
> > '/' from SLASH_MQ1, if for some reason that were preferred, but using
> > glibc functions when possible seems cleaner to me.
> >
> > Signed-off-by: Serge Hallyn <serue@us.ibm.com>
> > ---
> >  testcases/kernel/containers/mqns/mqns_01.c |    9 ++++-----
> >  testcases/kernel/containers/mqns/mqns_02.c |    8 +++-----
> >  testcases/kernel/containers/mqns/mqns_03.c |    3 +--
> >  testcases/kernel/containers/mqns/mqns_04.c |    2 +-
> >  4 files changed, 9 insertions(+), 13 deletions(-)
> >
> > diff --git a/testcases/kernel/containers/mqns/mqns_01.c b/testcases/kernel/containers/mqns/mqns_01.c
> > index 7f41b2d..2f3bf8e 100644
> > --- a/testcases/kernel/containers/mqns/mqns_01.c
> > +++ b/testcases/kernel/containers/mqns/mqns_01.c
> > @@ -55,7 +55,7 @@ int check_mqueue(void *vtest)
> >
> >        if (read(p1[0], buf, strlen("go") + 1) < 0)
> >                tst_resm(TBROK | TERRNO, "read(p1[0], ...) failed");
> > -       mqd = syscall(__NR_mq_open, SLASH_MQ1, O_RDONLY);
> > +       mqd = mq_open(SLASH_MQ1, O_RDONLY);
> >        if (mqd == -1) {
> >                if (write(p2[1], "notfnd", strlen("notfnd") + 1) < 0)
> >                        tst_resm(TBROK | TERRNO, "write(p2[1], ...) failed");
> > @@ -86,8 +86,7 @@ main(int argc, char *argv[])
> >        if (pipe(p1) == -1) { perror("pipe"); exit(EXIT_FAILURE); }
> >        if (pipe(p2) == -1) { perror("pipe"); exit(EXIT_FAILURE); }
> >
> > -       mqd = syscall(__NR_mq_open, SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0777,
> > -                       NULL);
> > +       mqd = mq_open(SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
> >        if (mqd == -1) {
> >                perror("mq_open");
> >                tst_resm(TFAIL, "mq_open failed\n");
> > @@ -100,7 +99,7 @@ main(int argc, char *argv[])
> >        if (r < 0) {
> >                tst_resm(TFAIL, "failed clone/unshare\n");
> >                mq_close(mqd);
> > -               syscall(__NR_mq_unlink, SLASH_MQ1);
> > +               mq_unlink(SLASH_MQ1);
> >                tst_exit();
> >        }
> >
> > @@ -122,7 +121,7 @@ main(int argc, char *argv[])
> >
> >        /* destroy the mqueue */
> >        mq_close(mqd);
> > -       syscall(__NR_mq_unlink, SLASH_MQ1);
> > +       mq_unlink(SLASH_MQ1);
> >
> >        tst_exit();
> >  }
> > diff --git a/testcases/kernel/containers/mqns/mqns_02.c b/testcases/kernel/containers/mqns/mqns_02.c
> > index aa78f65..5343d5b 100644
> > --- a/testcases/kernel/containers/mqns/mqns_02.c
> > +++ b/testcases/kernel/containers/mqns/mqns_02.c
> > @@ -60,8 +60,7 @@ int check_mqueue(void *vtest)
> >                tst_resm(TBROK | TERRNO, "read(p1[0], ..) failed");
> >        else {
> >
> > -               mqd = syscall(__NR_mq_open, SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL,
> > -                               0777, NULL);
> > +               mqd = mq_open(SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
> >                if (mqd == -1) {
> >                        if (write(p2[1], "mqfail", strlen("mqfail") + 1) < 0) {
> >                                tst_resm(TBROK | TERRNO,
> > @@ -85,8 +84,7 @@ int check_mqueue(void *vtest)
> >                                        if (mq_close(mqd) < 0) {
> >                                                tst_resm(TBROK | TERRNO,
> >                                                        "mq_close(mqd) failed");
> > -                                       } else if (syscall(__NR_mq_unlink,
> > -                                                       SLASH_MQ1) < 0) {
> > +                                       } else if (mq_unlink(SLASH_MQ1) < 0) {
> >                                                tst_resm(TBROK | TERRNO,
> >                                                        "mq_unlink(" SLASH_MQ1
> >                                                        ") failed");
> > @@ -153,7 +151,7 @@ int main(int argc, char *argv[])
> >                tst_exit();
> >        } else {
> >
> > -               mqd = syscall(__NR_mq_open, SLASH_MQ1, O_RDONLY);
> > +               mqd = mq_open(SLASH_MQ1, O_RDONLY);
> >                if (mqd == -1) {
> >                        tst_resm(TPASS, "Parent process can't see the mqueue\n");
> >                } else {
> > diff --git a/testcases/kernel/containers/mqns/mqns_03.c b/testcases/kernel/containers/mqns/mqns_03.c
> > index 3c9e83e..6a841b8 100644
> > --- a/testcases/kernel/containers/mqns/mqns_03.c
> > +++ b/testcases/kernel/containers/mqns/mqns_03.c
> > @@ -63,8 +63,7 @@ int check_mqueue(void *vtest)
> >
> >        read(p1[0], buf, 3); /* go */
> >
> > -       mqd = syscall(__NR_mq_open, SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0755,
> > -                       NULL);
> > +       mqd = mq_open(SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0755, NULL);
> >        if (mqd == -1) {
> >                write(p2[1], "mqfail", 7);
> >                tst_exit();
> > diff --git a/testcases/kernel/containers/mqns/mqns_04.c b/testcases/kernel/containers/mqns/mqns_04.c
> > index 8a4a9c2..6ce9e34 100644
> > --- a/testcases/kernel/containers/mqns/mqns_04.c
> > +++ b/testcases/kernel/containers/mqns/mqns_04.c
> > @@ -59,7 +59,7 @@ int check_mqueue(void *vtest)
> >
> >        read(p1[0], buf, 3); /* go */
> >
> > -       mqd = syscall(__NR_mq_open, SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0755,
> > +       mqd = mq_open(SLASH_MQ1, O_RDWR|O_CREAT|O_EXCL, 0755,
> >                        NULL);
> >        if (mqd == -1) {
> >                write(p2[1], "mqfail", 7);
> 
>     The problem with this is that we're instead testing indirectly via
> libc instead of directly via syscall(2). Why not just have two

Yup - we're testing the namespaces here, not the mq_open syscall itself,
so I wasn't sure whether we minded inadvertently testing libc as well.
(The mq_open() also is cleaner to read)

> constants -- one that has a slash and one that doesn't? Seems like
> it'd be simple to implement...

<shrug>  yup, that's why I made sure to make that clear, that's the
other way, and will work just fine.

(I'm actually out for the rest of this week, back monday)

-serge

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev 
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list