Hi,
On Tue, Aug 4, 2015 at 9:45 PM, Cyril Hrubis <chru...@suse.cz> wrote:
> Hi!
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/ipc/msgrcv/msgrcv08.c
> > @@ -0,0 +1,135 @@
> > +/*
> > + * Copyright (c) 2015 Author: Gabriellla Schmidt <g...@bruker.de>
> > + * Modify: Li Wang <liw...@redhat.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> it
> > + * under the terms of version 2 of the GNU General Public License as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it would be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> > + *
> > + * you should have received a copy of the GNU General Public License
> along
> > + * with this program; if not, write the Free Software Foundation, Inc.,
> > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> > + */
> > +
> > +/*
> > + * Description:
> > + *
> > + * A regression test for:
> > + * commit e7ca2552369c1dfe0216c626baf82c3d83ec36bb
> > + * Author: Mateusz Guzik <mgu...@redhat.com>
> > + * Date: Mon Jan 27 17:07:11 2014 -0800
> > + *
> > + * ipc: fix compat msgrcv with negative msgtyp
> > + *
> > + * Reproduce:
> > + *
> > + * 32-bit application using the msgrcv() system call
> > + * gives the error message:
> > + *
> > + * msgrcv: No message of desired type
> > + *
> > + * If this progarm is compiled as 64-bit application it works.
> > + */
> > +
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +#include <sys/ipc.h>
> > +#include <sys/msg.h>
> > +#include "test.h"
> > +
> > +const char *TCID = "msgrcv08";
> > +const int TST_TOTAL = 1;
> > +
> > +#if __WORDSIZE == 32
> > +
> > +struct msgbuf {
> > + long mtype; /* message type, must be > 0 */
> > + char mtext[16]; /* message data */
> > +};
> > +
> > +static void setup(void)
> > +{
> > + tst_require_root(NULL);
>
> Do we really need to be root to run the testcase?
>
ok, actually we don't, will remove it.
>
> > + TEST_PAUSE;
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +}
>
> No empty cleanups please. If there is nothing to be cleaned up simply do
> not implement the function at all.
>
will remove cleanup().
>
> > +static int msr(int msqid)
> > +{
> > + struct msgbuf msbs;
> > + struct msgbuf msbr;
> > + ssize_t sret;
> > + long mtype = 121;
> ^
> Only single space here please.
>
ok.
>
> > + memset(&msbs, 0, sizeof(msbs));
> > + msbs.mtype = mtype;
> > +
> > + if (msgsnd(msqid, &msbs, sizeof(msbs.mtext), IPC_NOWAIT))
> > + tst_brkm(TBROK, NULL, "msgsnd error");
> ^
> Should be TBROK | TERRNO so that we know
> the reason of the failure.
>
ok.
>
> > +
> > + sret = msgrcv(msqid, &msbr, sizeof(msbr.mtext), -mtype, IPC_NOWAIT
> | MSG_NOERROR);
> > +
> > + if (sret < 0) {
> > + tst_resm(TFAIL, "Bug: No message of desired type.");
> > + return -1;
> > + }
> > +
> > + if (msbr.mtype != mtype)
> > + tst_brkm(TBROK, NULL,
> > + "found mtype %ld, expected %ld\n", msbr.mtype,
> mtype);
> > +
> > + if ((size_t)sret != sizeof(msbs.mtext))
> > + tst_brkm(TBROK, NULL, "received %lu, expected %lu\n",
> > + (unsigned long)sret, (unsigned
> long)sizeof(msbs.mtext));
>
> Use %zi and %zu instead of the %lu and drop the casts to unsigned long.
>
great! thanks for pointing this out.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void msgrcv_test(void)
> > +{
> > + int ret;
> > + int msqid = msgget(IPC_PRIVATE, IPC_CREAT | IPC_EXCL | 0666);
> > +
> > + if (msqid < 0)
> > + tst_brkm(TBROK, NULL, "msgget error");
> ^
> Add the TERRNO here as well
>
> > + ret = msr(msqid);
> > +
> > + if (msgctl(msqid, IPC_RMID, 0))
> > + tst_brkm(TBROK, NULL, "msgctl error");
> ^
> And here as well.
>
> > + if (!ret)
> > + tst_resm(TPASS, "Hi, no regression found!");
>
> What about instead of passing the value here to print the TPASS message
> we call it at the end of the msr() function instead of doing return 0; ?
>
Yeah, that would be better.
>
> Also please drop the "Hi, " from the message ;).
>
:)
>
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + int lc;
> > +
> > + tst_parse_opts(argc, argv, NULL, NULL);
> > +
> > + setup();
> > +
> > + for (lc = 0; TEST_LOOPING(lc); lc++)
> > + msgrcv_test();
> > +
> > + cleanup();
> > + tst_exit();
> > +}
> > +
> > +#else /* no 64-bit */
> > +int main(void)
> > +{
> > + tst_brkm(TCONF, NULL, "not works when compiled as 64-bit
> application.");
> ^
> Please use tabs for indentation only.
>
ok.
>
> > +}
> > +#endif
> > --
> > 1.8.3.1
> >
> >
> >
> ------------------------------------------------------------------------------
> > _______________________________________________
> > Ltp-list mailing list
> > Ltp-list@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/ltp-list
>
> --
> Cyril Hrubis
> chru...@suse.cz
>
Thanks for review, I will post V2.
--
Regards,
Li Wang
Email: liw...@redhat.com
------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
