[LTP] [PATCH v1] Move semaphore/ tests to semop/

[LTP] [PATCH v1] Move semaphore/ tests to semop/

[Wei Gao via ltp]

There is no point in having two different directories with semop()
testcaes so it would make sense to move these two tests into the
syscalls/ipc/semop/ directory as semop04 and semop05.
So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
Also update runtest files accordingly.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/syscalls                              |  2 ++
 testcases/kernel/ipc/semaphore/.gitignore     |  2 --
 testcases/kernel/ipc/semaphore/Makefile       | 30 -------------------
 .../kernel/syscalls/ipc/semop/.gitignore      |  2 ++
 testcases/kernel/syscalls/ipc/semop/Makefile  |  1 +
 .../sem01.c => syscalls/ipc/semop/semop04.c}  |  0
 .../sem02.c => syscalls/ipc/semop/semop05.c}  |  0
 7 files changed, 5 insertions(+), 32 deletions(-)
 delete mode 100644 testcases/kernel/ipc/semaphore/.gitignore
 delete mode 100644 testcases/kernel/ipc/semaphore/Makefile
 rename testcases/kernel/{ipc/semaphore/sem01.c => syscalls/ipc/semop/semop04.c} (100%)
 rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

diff --git a/runtest/syscalls b/runtest/syscalls
index ae37a1192..54cbc6ece 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1239,6 +1239,8 @@ semget06 semget06
 semop01 semop01
 semop02 semop02
 semop03 semop03
+semop04 semop04
+semop05 semop05
 
 send01 send01
 send02 send02
diff --git a/testcases/kernel/ipc/semaphore/.gitignore b/testcases/kernel/ipc/semaphore/.gitignore
deleted file mode 100644
index 944e5ca1e..000000000
--- a/testcases/kernel/ipc/semaphore/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-/sem01
-/sem02
diff --git a/testcases/kernel/ipc/semaphore/Makefile b/testcases/kernel/ipc/semaphore/Makefile
deleted file mode 100644
index 9ab85745d..000000000
--- a/testcases/kernel/ipc/semaphore/Makefile
+++ /dev/null
@@ -1,30 +0,0 @@
-#
-#    testcases/kernel/ipc/semaphore Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir		?= ../../../..
-
-include $(top_srcdir)/include/mk/testcases.mk
-
-CPPFLAGS 		+= -I../include
-LDLIBS			+= -lpthread
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/ipc/semop/.gitignore b/testcases/kernel/syscalls/ipc/semop/.gitignore
index bb57f08af..cc67b1862 100644
--- a/testcases/kernel/syscalls/ipc/semop/.gitignore
+++ b/testcases/kernel/syscalls/ipc/semop/.gitignore
@@ -1,3 +1,5 @@
 /semop01
 /semop02
 /semop03
+/semop04
+/semop05
diff --git a/testcases/kernel/syscalls/ipc/semop/Makefile b/testcases/kernel/syscalls/ipc/semop/Makefile
index 6b2b26d05..8ee2f7fe6 100644
--- a/testcases/kernel/syscalls/ipc/semop/Makefile
+++ b/testcases/kernel/syscalls/ipc/semop/Makefile
@@ -8,5 +8,6 @@ LTPLIBS = ltpnewipc
 include $(top_srcdir)/include/mk/testcases.mk
 
 LTPLDLIBS  = -lltpnewipc
+LDLIBS    += -lpthread
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/ipc/semaphore/sem01.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem01.c
rename to testcases/kernel/syscalls/ipc/semop/semop04.c
diff --git a/testcases/kernel/ipc/semaphore/sem02.c b/testcases/kernel/syscalls/ipc/semop/semop05.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem02.c
rename to testcases/kernel/syscalls/ipc/semop/semop05.c
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] Move semaphore/ tests to semop/

[Petr Vorel]

Hi Wei,

> There is no point in having two different directories with semop()
> testcaes so it would make sense to move these two tests into the
> syscalls/ipc/semop/ directory as semop04 and semop05.
> So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> Also update runtest files accordingly.

testcases/kernel/ipc/semaphore/Makefile
links with libpthread.so.0:
LDLIBS			+= -lpthread

Although it looks like only sem02.c uses pthread functions.
https://man7.org/linux/man-pages/man7/pthreads.7.html

testcases/kernel/syscalls/ipc/semop/Makefile
links with ltp library libltpnewipc.a:
LTPLDLIBS  = -lltpnewipc
which uses /proc/sysvipc/
https://man7.org/linux/man-pages/man7/sysvipc.7.html

I'm not sure if it's good idea to mix both
@Cyril any input?

> +++ b/testcases/kernel/syscalls/ipc/semop/Makefile
> @@ -8,5 +8,6 @@ LTPLIBS = ltpnewipc
>  include $(top_srcdir)/include/mk/testcases.mk

>  LTPLDLIBS  = -lltpnewipc
> +LDLIBS    += -lpthread

If it's ok to merge them into single directory, it'd be better to specify
used libraries for files which need them. e.g.:

semop01: LTPLDLIBS  = -lltpnewipc
semop02: LTPLDLIBS  = -lltpnewipc
semop03: LTPLDLIBS  = -lltpnewipc
semop05: LDLIBS    += -lpthread

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] Move semaphore/ tests to semop/

[Wei Gao via ltp]

There is no point in having two different directories with semop()
testcaes so it would make sense to move these two tests into the
syscalls/ipc/semop/ directory as semop04 and semop05.
So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
Also update runtest files accordingly.

Signed-off-by: Wei Gao <wegao@suse.com>
---
v2->v1:Update Makefile give each case specific link option

 runtest/syscalls                              |  2 ++
 testcases/kernel/ipc/semaphore/.gitignore     |  2 --
 testcases/kernel/ipc/semaphore/Makefile       | 30 -------------------
 .../kernel/syscalls/ipc/semop/.gitignore      |  2 ++
 testcases/kernel/syscalls/ipc/semop/Makefile  |  5 +++-
 .../sem01.c => syscalls/ipc/semop/semop04.c}  |  0
 .../sem02.c => syscalls/ipc/semop/semop05.c}  |  0
 7 files changed, 8 insertions(+), 33 deletions(-)
 delete mode 100644 testcases/kernel/ipc/semaphore/.gitignore
 delete mode 100644 testcases/kernel/ipc/semaphore/Makefile
 rename testcases/kernel/{ipc/semaphore/sem01.c => syscalls/ipc/semop/semop04.c} (100%)
 rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

diff --git a/runtest/syscalls b/runtest/syscalls
index ae37a1192..54cbc6ece 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1239,6 +1239,8 @@ semget06 semget06
 semop01 semop01
 semop02 semop02
 semop03 semop03
+semop04 semop04
+semop05 semop05
 
 send01 send01
 send02 send02
diff --git a/testcases/kernel/ipc/semaphore/.gitignore b/testcases/kernel/ipc/semaphore/.gitignore
deleted file mode 100644
index 944e5ca1e..000000000
--- a/testcases/kernel/ipc/semaphore/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-/sem01
-/sem02
diff --git a/testcases/kernel/ipc/semaphore/Makefile b/testcases/kernel/ipc/semaphore/Makefile
deleted file mode 100644
index 9ab85745d..000000000
--- a/testcases/kernel/ipc/semaphore/Makefile
+++ /dev/null
@@ -1,30 +0,0 @@
-#
-#    testcases/kernel/ipc/semaphore Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir		?= ../../../..
-
-include $(top_srcdir)/include/mk/testcases.mk
-
-CPPFLAGS 		+= -I../include
-LDLIBS			+= -lpthread
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/ipc/semop/.gitignore b/testcases/kernel/syscalls/ipc/semop/.gitignore
index bb57f08af..cc67b1862 100644
--- a/testcases/kernel/syscalls/ipc/semop/.gitignore
+++ b/testcases/kernel/syscalls/ipc/semop/.gitignore
@@ -1,3 +1,5 @@
 /semop01
 /semop02
 /semop03
+/semop04
+/semop05
diff --git a/testcases/kernel/syscalls/ipc/semop/Makefile b/testcases/kernel/syscalls/ipc/semop/Makefile
index 6b2b26d05..43afffb3f 100644
--- a/testcases/kernel/syscalls/ipc/semop/Makefile
+++ b/testcases/kernel/syscalls/ipc/semop/Makefile
@@ -7,6 +7,9 @@ LTPLIBS = ltpnewipc
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-LTPLDLIBS  = -lltpnewipc
+semop01: LTPLDLIBS  = -lltpnewipc
+semop02: LTPLDLIBS  = -lltpnewipc
+semop03: LTPLDLIBS  = -lltpnewipc
+semop05: LDLIBS    += -lpthread
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/ipc/semaphore/sem01.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem01.c
rename to testcases/kernel/syscalls/ipc/semop/semop04.c
diff --git a/testcases/kernel/ipc/semaphore/sem02.c b/testcases/kernel/syscalls/ipc/semop/semop05.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem02.c
rename to testcases/kernel/syscalls/ipc/semop/semop05.c
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] Move semaphore/ tests to semop/

[Richard Palethorpe]

Hello,

Wei Gao via ltp <ltp@lists.linux.it> writes:

> There is no point in having two different directories with semop()
> testcaes so it would make sense to move these two tests into the
> syscalls/ipc/semop/ directory as semop04 and semop05.
> So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> Also update runtest files accordingly.

Yes, true. However these tests are using the old API. It makes sense to
move them and update to the new API at the same time. Maybe they even
test the same things as the other tests, so they could be deduplicated.

I like breaking things into small tasks, but this is actually too
small. I think it would be better to convert and move one test at a time
as well (in separate commits, but in the same patch series).

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 0/2] Refactor semaphore

[Wei Gao via ltp]

Wei Gao (2):
  Move semaphore/ tests to semop/
  semop04:Refactor with new API

 runtest/syscalls                              |   2 +
 testcases/kernel/ipc/semaphore/.gitignore     |   2 -
 testcases/kernel/ipc/semaphore/Makefile       |  30 ----
 testcases/kernel/ipc/semaphore/sem01.c        | 164 ------------------
 .../kernel/syscalls/ipc/semop/.gitignore      |   2 +
 testcases/kernel/syscalls/ipc/semop/Makefile  |   5 +-
 testcases/kernel/syscalls/ipc/semop/semop04.c | 137 +++++++++++++++
 .../sem02.c => syscalls/ipc/semop/semop05.c}  |   0
 8 files changed, 145 insertions(+), 197 deletions(-)
 delete mode 100644 testcases/kernel/ipc/semaphore/.gitignore
 delete mode 100644 testcases/kernel/ipc/semaphore/Makefile
 delete mode 100644 testcases/kernel/ipc/semaphore/sem01.c
 create mode 100644 testcases/kernel/syscalls/ipc/semop/semop04.c
 rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 1/2] Move semaphore/ tests to semop/

[Wei Gao via ltp]

There is no point in having two different directories with semop()
testcaes so it would make sense to move these two tests into the
syscalls/ipc/semop/ directory as semop04 and semop05.
So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
Also update runtest files accordingly.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/syscalls                              |  2 ++
 testcases/kernel/ipc/semaphore/.gitignore     |  2 --
 testcases/kernel/ipc/semaphore/Makefile       | 30 -------------------
 .../kernel/syscalls/ipc/semop/.gitignore      |  2 ++
 testcases/kernel/syscalls/ipc/semop/Makefile  |  5 +++-
 .../sem01.c => syscalls/ipc/semop/semop04.c}  |  0
 .../sem02.c => syscalls/ipc/semop/semop05.c}  |  0
 7 files changed, 8 insertions(+), 33 deletions(-)
 delete mode 100644 testcases/kernel/ipc/semaphore/.gitignore
 delete mode 100644 testcases/kernel/ipc/semaphore/Makefile
 rename testcases/kernel/{ipc/semaphore/sem01.c => syscalls/ipc/semop/semop04.c} (100%)
 rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

diff --git a/runtest/syscalls b/runtest/syscalls
index ae37a1192..54cbc6ece 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1239,6 +1239,8 @@ semget06 semget06
 semop01 semop01
 semop02 semop02
 semop03 semop03
+semop04 semop04
+semop05 semop05
 
 send01 send01
 send02 send02
diff --git a/testcases/kernel/ipc/semaphore/.gitignore b/testcases/kernel/ipc/semaphore/.gitignore
deleted file mode 100644
index 944e5ca1e..000000000
--- a/testcases/kernel/ipc/semaphore/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-/sem01
-/sem02
diff --git a/testcases/kernel/ipc/semaphore/Makefile b/testcases/kernel/ipc/semaphore/Makefile
deleted file mode 100644
index 9ab85745d..000000000
--- a/testcases/kernel/ipc/semaphore/Makefile
+++ /dev/null
@@ -1,30 +0,0 @@
-#
-#    testcases/kernel/ipc/semaphore Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir		?= ../../../..
-
-include $(top_srcdir)/include/mk/testcases.mk
-
-CPPFLAGS 		+= -I../include
-LDLIBS			+= -lpthread
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/ipc/semop/.gitignore b/testcases/kernel/syscalls/ipc/semop/.gitignore
index bb57f08af..cc67b1862 100644
--- a/testcases/kernel/syscalls/ipc/semop/.gitignore
+++ b/testcases/kernel/syscalls/ipc/semop/.gitignore
@@ -1,3 +1,5 @@
 /semop01
 /semop02
 /semop03
+/semop04
+/semop05
diff --git a/testcases/kernel/syscalls/ipc/semop/Makefile b/testcases/kernel/syscalls/ipc/semop/Makefile
index 6b2b26d05..43afffb3f 100644
--- a/testcases/kernel/syscalls/ipc/semop/Makefile
+++ b/testcases/kernel/syscalls/ipc/semop/Makefile
@@ -7,6 +7,9 @@ LTPLIBS = ltpnewipc
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-LTPLDLIBS  = -lltpnewipc
+semop01: LTPLDLIBS  = -lltpnewipc
+semop02: LTPLDLIBS  = -lltpnewipc
+semop03: LTPLDLIBS  = -lltpnewipc
+semop05: LDLIBS    += -lpthread
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/ipc/semaphore/sem01.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem01.c
rename to testcases/kernel/syscalls/ipc/semop/semop04.c
diff --git a/testcases/kernel/ipc/semaphore/sem02.c b/testcases/kernel/syscalls/ipc/semop/semop05.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem02.c
rename to testcases/kernel/syscalls/ipc/semop/semop05.c
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/2] Move semaphore/ tests to semop/

[Petr Vorel]

Hi Wei,

> There is no point in having two different directories with semop()
> testcaes so it would make sense to move these two tests into the
> syscalls/ipc/semop/ directory as semop04 and semop05.
> So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> Also update runtest files accordingly.

As I already stated, I'm not really sure this is good to do, but maybe I'm
missing something.  I wonder what others thing about it.

...
>  rename testcases/kernel/{ipc/semaphore/sem01.c => syscalls/ipc/semop/semop04.c} (100%)
>  rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

> +++ b/runtest/syscalls
> @@ -1239,6 +1239,8 @@ semget06 semget06
>  semop01 semop01
>  semop02 semop02
>  semop03 semop03
> +semop04 semop04
> +semop05 semop05
You renamed files, you need to remove sem01 sem02 from runtest/ipc.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/2] Move semaphore/ tests to semop/

[Wei Gao via ltp]

On Wed, Mar 15, 2023 at 01:31:19PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> > There is no point in having two different directories with semop()
> > testcaes so it would make sense to move these two tests into the
> > syscalls/ipc/semop/ directory as semop04 and semop05.
> > So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> > Also update runtest files accordingly.
> 
> As I already stated, I'm not really sure this is good to do, but maybe I'm
> missing something.  I wonder what others thing about it.
> 
Ah, you also start work on this?  Should i still continue work on this patch or not?
To me this is not bad thing at least, we do some clean work and reorganize the test cases.
> ...
> >  rename testcases/kernel/{ipc/semaphore/sem01.c => syscalls/ipc/semop/semop04.c} (100%)
> >  rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)
> 
> > +++ b/runtest/syscalls
> > @@ -1239,6 +1239,8 @@ semget06 semget06
> >  semop01 semop01
> >  semop02 semop02
> >  semop03 semop03
> > +semop04 semop04
> > +semop05 semop05
> You renamed files, you need to remove sem01 sem02 from runtest/ipc.
Ack!
> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/2] Move semaphore/ tests to semop/

[Petr Vorel]

Hi Wei,

> > > There is no point in having two different directories with semop()
> > > testcaes so it would make sense to move these two tests into the
> > > syscalls/ipc/semop/ directory as semop04 and semop05.
> > > So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> > > Also update runtest files accordingly.

> > As I already stated, I'm not really sure this is good to do, but maybe I'm
> > missing something.  I wonder what others thing about it.

> Ah, you also start work on this?
No, I just shared my doubt about this move.

> Should i still continue work on this patch or not?
> To me this is not bad thing at least, we do some clean work and reorganize the test cases.

Probably yes, but I'd prefer somebody else from LTP maintainers has also acked this.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/2] Move semaphore/ tests to semop/

[Li Wang]

Petr Vorel <pvorel@suse.cz> wrote:

Hi Wei,
>
> > There is no point in having two different directories with semop()
> > testcaes so it would make sense to move these two tests into the
> > syscalls/ipc/semop/ directory as semop04 and semop05.
> > So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> > Also update runtest files accordingly.
>


> As I already stated, I'm not really sure this is good to do, but maybe I'm
> missing something.  I wonder what others think about it.
>

This patch makes sense. Petr, I know what you're concerned about,
but it is correct to move them into syscalls/ipc/semop/.

They essentially all belong to System V IPC way to use
semaphore which includes <sys/sem.h>:
  semid_ds
  semget()
  semctl()
  semop()

To distinguish from another POSIX IPC way which
include <semaphore.h>:
  sem_open()
  sem_close()
  sem_unlink()
  sem_post(), sem_wait()
  sem_getvalue()
  sem_init()
  sem_destrory()

The sem02.c needs '-lpthread' only because it invoked pthread_attr_init/exit
functions in the test, but not any POSIX IPC functions.

So you have my reviewed-by in patch v4:
Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 1/2] Move semaphore/ tests to semop/

[Petr Vorel]

> Petr Vorel <pvorel@suse.cz> wrote:

> Hi Wei,

> > > There is no point in having two different directories with semop()
> > > testcaes so it would make sense to move these two tests into the
> > > syscalls/ipc/semop/ directory as semop04 and semop05.
> > > So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
> > > Also update runtest files accordingly.



> > As I already stated, I'm not really sure this is good to do, but maybe I'm
> > missing something.  I wonder what others think about it.


> This patch makes sense. Petr, I know what you're concerned about,
> but it is correct to move them into syscalls/ipc/semop/.

> They essentially all belong to System V IPC way to use
> semaphore which includes <sys/sem.h>:
>   semid_ds
>   semget()
>   semctl()
>   semop()

> To distinguish from another POSIX IPC way which
> include <semaphore.h>:
>   sem_open()
>   sem_close()
>   sem_unlink()
>   sem_post(), sem_wait()
>   sem_getvalue()
>   sem_init()
>   sem_destrory()

> The sem02.c needs '-lpthread' only because it invoked pthread_attr_init/exit
> functions in the test, but not any POSIX IPC functions.

> So you have my reviewed-by in patch v4:
> Reviewed-by: Li Wang <liwang@redhat.com>

Li, thanks for having look, first patch in merged v4.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 2/2] semop04:Refactor with new API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ipc/semop/semop04.c | 163 ++++++++----------
 1 file changed, 68 insertions(+), 95 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
index 582624d60..dab34075c 100644
--- a/testcases/kernel/syscalls/ipc/semop/semop04.c
+++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
@@ -1,36 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (C) 2023 Linux Test Project, Inc.
  */
 
-/*
- *  FILE        : sem01.c
- *  DESCRIPTION : Creates a semaphore and two processes.  The processes
- *                each go through a loop where they semdown, delay for a
- *                random amount of time, and semup, so they will almost
- *                always be fighting for control of the semaphore.
- *  HISTORY:
- *    01/15/2001 Paul Larson (plars@us.ibm.com)
- *      -written
- *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
- *    Modified.
- *    - Removed compiler warnings.
- *      added exit to the end of function main()
+/*\
+ * [Description]
  *
+ * Creates a semaphore and two processes.  The processes
+ * each go through a loop where they semdown, delay for a
+ * random amount of time, and semup, so they will almost
+ * always be fighting for control of the semaphore.
  */
 
 #include <unistd.h>
@@ -41,103 +21,85 @@
 #include <sys/wait.h>
 #include <sys/ipc.h>
 #include "lapi/sem.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 
-int verbose = 0;
-int loops = 100;
-int errors = 0;
+static char *verbose;
+static int loops = 100;
+static char *opt_loops_str;
 
-int semup(int semid)
+static void semup(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
+
 	semops.sem_op = 1;
+
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semup");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-int semdown(int semid)
+static void semdown(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
+
 	semops.sem_op = -1;
+
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semdown");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-void delayloop()
+static void delayloop(void)
 {
 	int delay;
+
 	delay = 1 + ((100.0 * rand()) / RAND_MAX);
+
 	if (verbose)
-		printf("in delay function for %d microseconds\n", delay);
+		tst_res(TINFO, "in delay function for %d microseconds", delay);
 	usleep(delay);
 }
 
 void mainloop(int semid)
 {
 	int i;
+
 	for (i = 0; i < loops; i++) {
-		if (semdown(semid)) {
-			printf("semdown failed\n");
-		}
+		semdown(semid);
 		if (verbose)
-			printf("sem is down\n");
+			tst_res(TINFO, "Sem is down");
 		delayloop();
-		if (semup(semid)) {
-			printf("semup failed\n");
-		}
+		semup(semid);
 		if (verbose)
-			printf("sem is up\n");
+			tst_res(TINFO, "Sem is up");
 	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int semid, opt;
+	int semid;
 	union semun semunion;
-	extern char *optarg;
 	pid_t pid;
 	int chstat;
 
-	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
-		switch ((char)opt) {
-		case 'l':
-			loops = atoi(optarg);
-			break;
-		case 'v':
-			verbose = 1;
-			break;
-		case 'h':
-		default:
-			printf("Usage: -l loops [-v]\n");
-			exit(1);
-		}
-	}
-
 	/* set up the semaphore */
-	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
-		printf("error in semget()\n");
-		exit(-1);
-	}
+	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
+	if (semid < 0)
+		tst_brk(TBROK, "Error in semget id=%i", semid);
+
 	semunion.val = 1;
-	if (semctl(semid, 0, SETVAL, semunion) == -1) {
-		printf("error in semctl\n");
-	}
 
-	if ((pid = fork()) < 0) {
-		printf("fork error\n");
-		exit(-1);
-	}
+	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
+
+	pid = SAFE_FORK();
+	if (pid < 0)
+		tst_brk(TBROK, "Fork failed pid %i", pid);
+
 	if (pid) {
 		/* parent */
 		srand(pid);
@@ -147,18 +109,29 @@ int main(int argc, char *argv[])
 			printf("child exited with status\n");
 			exit(-1);
 		}
-		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
-			printf("error in semctl\n");
-		}
-		if (errors) {
-			printf("FAIL: there were %d errors\n", errors);
-		} else {
-			printf("PASS: error count is 0\n");
-		}
-		exit(errors);
+		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
+		tst_res(TPASS, "Semaphore up/down check success");
 	} else {
 		/* child */
 		mainloop(semid);
 	}
-	exit(0);
 }
+
+static void setup(void)
+{
+	if (opt_loops_str)
+		loops = SAFE_STRTOL(opt_loops_str, 1, INT_MAX);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{"l:", &opt_loops_str, "Internal loops for semup/down"},
+		{"v", &verbose,
+			"Print information about successful semop."},
+		{}
+	},
+	.needs_root = 1,
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/2] semop04:Refactor with new API

[Petr Vorel]

Hi Wei,

...
> -int verbose = 0;
> -int loops = 100;
> -int errors = 0;
> +static char *verbose;

We usually prefer to not introduce verbose option in refactored tests.
It'd be nice to have output, which is not too verbose, but not just TPASS
message (something in between).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/2] semop04:Refactor with new API

[Wei Gao via ltp]

On Wed, Mar 15, 2023 at 01:36:41PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> ...
> > -int verbose = 0;
> > -int loops = 100;
> > -int errors = 0;
> > +static char *verbose;
> 
> We usually prefer to not introduce verbose option in refactored tests.
> It'd be nice to have output, which is not too verbose, but not just TPASS
> message (something in between).
I just need double confirm with your idea:
Remove this option and let current verbose info print out by default?
> 
> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/2] semop04:Refactor with new API

[Petr Vorel]

Hi Wei,

> +// SPDX-License-Identifier: GPL-2.0
Below is "or (at your option) any later version."
=> 
// SPDX-License-Identifier: GPL-2.0-or-later

>  /*
> - *
> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   This program is free software;  you can redistribute it and/or modify
> - *   it under the terms of the GNU General Public License as published by
> - *   the Free Software Foundation; either version 2 of the License, or
> - *   (at your option) any later version.
> - *
> - *   This program is distributed in the hope that it will be useful,
> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - *   the GNU General Public License for more details.
> - *
> - *   You should have received a copy of the GNU General Public License
> - *   along with this program;  if not, write to the Free Software
> - *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * Copyright (c) International Business Machines  Corp., 2001
> + * Copyright (C) 2023 Linux Test Project, Inc.
Actually LTP started to work on the file since 2003, i.e.
* Copyright (C) 2003-2023 Linux Test Project, Inc.
>   */

> -/*
> - *  FILE        : sem01.c
> - *  DESCRIPTION : Creates a semaphore and two processes.  The processes
> - *                each go through a loop where they semdown, delay for a
> - *                random amount of time, and semup, so they will almost
> - *                always be fighting for control of the semaphore.
> - *  HISTORY:
> - *    01/15/2001 Paul Larson (plars@us.ibm.com)
> - *      -written
> - *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
Probably you should keep these two names (although you can clean formatting a
bit), e.g.:
* Author: 2001 Paul Larson <plars@us.ibm.com>
* Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>

> - *    Modified.
> - *    - Removed compiler warnings.
> - *      added exit to the end of function main()

...
> -int semup(int semid)
> +static void semup(int semid)
>  {
>  	struct sembuf semops;
> +
>  	semops.sem_num = 0;
> +
nit: I'd remove this blank line (readability)
>  	semops.sem_op = 1;
> +
>  	semops.sem_flg = SEM_UNDO;
> -	if (semop(semid, &semops, 1) == -1) {
> -		perror("semup");
> -		errors++;
> -		return 1;
> -	}
> -	return 0;
> +
> +	SAFE_SEMOP(semid, &semops, 1);
>  }

> -int semdown(int semid)
> +static void semdown(int semid)
>  {
>  	struct sembuf semops;
> +
>  	semops.sem_num = 0;
> +
nit: also here remove blank line.
>  	semops.sem_op = -1;
> +
>  	semops.sem_flg = SEM_UNDO;
> -	if (semop(semid, &semops, 1) == -1) {
> -		perror("semdown");
> -		errors++;
> -		return 1;
> -	}
> -	return 0;
> +
> +	SAFE_SEMOP(semid, &semops, 1);
>  }

> -void delayloop()
> +static void delayloop(void)
>  {
>  	int delay;
> +
>  	delay = 1 + ((100.0 * rand()) / RAND_MAX);
> +
>  	if (verbose)
> -		printf("in delay function for %d microseconds\n", delay);
> +		tst_res(TINFO, "in delay function for %d microseconds", delay);
I'd remove this message.

>  	usleep(delay);
>  }

>  void mainloop(int semid)
>  {
>  	int i;
> +
>  	for (i = 0; i < loops; i++) {
> -		if (semdown(semid)) {
> -			printf("semdown failed\n");
> -		}
> +		semdown(semid);


>  		if (verbose)
> -			printf("sem is down\n");
> +			tst_res(TINFO, "Sem is down");
Actually this message is IMHO useless. semdown() uses SAFE_SEMOP() => we will
see where it failed.

>  		delayloop();
> -		if (semup(semid)) {
> -			printf("semup failed\n");
> -		}
> +		semup(semid);
>  		if (verbose)
> -			printf("sem is up\n");
> +			tst_res(TINFO, "Sem is up");
The same applies there => I'd remove verbose and these messages.
>  	}
>  }

> -int main(int argc, char *argv[])
> +static void run(void)
>  {
> -	int semid, opt;
> +	int semid;
>  	union semun semunion;
> -	extern char *optarg;
>  	pid_t pid;
>  	int chstat;

> -	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
> -		switch ((char)opt) {
> -		case 'l':
> -			loops = atoi(optarg);
> -			break;
> -		case 'v':
> -			verbose = 1;
> -			break;
> -		case 'h':
> -		default:
> -			printf("Usage: -l loops [-v]\n");
> -			exit(1);
> -		}
> -	}
> -
>  	/* set up the semaphore */
> -	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
> -		printf("error in semget()\n");
> -		exit(-1);
> -	}
> +	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
> +	if (semid < 0)
> +		tst_brk(TBROK, "Error in semget id=%i", semid);
All safe messages handles error checking and call tst_brk() on error => this if
check is unreachable (useless) => remove it.
> +
>  	semunion.val = 1;
> -	if (semctl(semid, 0, SETVAL, semunion) == -1) {
> -		printf("error in semctl\n");
> -	}

> -	if ((pid = fork()) < 0) {
> -		printf("fork error\n");
> -		exit(-1);
> -	}
> +	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
> +
> +	pid = SAFE_FORK();
> +	if (pid < 0)
> +		tst_brk(TBROK, "Fork failed pid %i", pid);
Also this if check is useless => remove it.
> +
>  	if (pid) {
>  		/* parent */
>  		srand(pid);
> @@ -147,18 +109,29 @@ int main(int argc, char *argv[])
>  			printf("child exited with status\n");
>  			exit(-1);
>  		}
NOTE: waitpid(), WIFEXITED(), printf() and exit(-1) here should be replaced by tst_reap_children(), see
https://github.com/linux-test-project/ltp/wiki/C-Test-API#18-doing-the-test-in-the-child-process
91e6a95b7 ("clone08: convert to new LTP API")

> +		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
> +		tst_res(TPASS, "Semaphore up/down check success");
>  	} else {
>  		/* child */
>  		mainloop(semid);
>  	}
> -	exit(0);
>  }
> +
> +static void setup(void)
> +{
> +	if (opt_loops_str)
> +		loops = SAFE_STRTOL(opt_loops_str, 1, INT_MAX);
> +}
I wonder if we need to specify loops, we have -i which can do more runs.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 0/2] Refactor semaphore

[Wei Gao via ltp]

Wei Gao (2):
  Move semaphore/ tests to semop/
  semop04:Refactor with new API

 runtest/ipc                                   |   2 -
 runtest/syscalls                              |   2 +
 testcases/kernel/ipc/semaphore/.gitignore     |   2 -
 testcases/kernel/ipc/semaphore/Makefile       |  30 ----
 testcases/kernel/ipc/semaphore/sem01.c        | 164 ------------------
 .../kernel/syscalls/ipc/semop/.gitignore      |   2 +
 testcases/kernel/syscalls/ipc/semop/Makefile  |   5 +-
 testcases/kernel/syscalls/ipc/semop/semop04.c | 117 +++++++++++++
 .../sem02.c => syscalls/ipc/semop/semop05.c}  |   0
 9 files changed, 125 insertions(+), 199 deletions(-)
 delete mode 100644 testcases/kernel/ipc/semaphore/.gitignore
 delete mode 100644 testcases/kernel/ipc/semaphore/Makefile
 delete mode 100644 testcases/kernel/ipc/semaphore/sem01.c
 create mode 100644 testcases/kernel/syscalls/ipc/semop/semop04.c
 rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 1/2] Move semaphore/ tests to semop/

[Wei Gao via ltp]

There is no point in having two different directories with semop()
testcaes so it would make sense to move these two tests into the
syscalls/ipc/semop/ directory as semop04 and semop05.
So we move ipc/semaphore/ tests to syscalls/ipc/semop/.
Also update runtest files accordingly.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 runtest/ipc                                   |  2 --
 runtest/syscalls                              |  2 ++
 testcases/kernel/ipc/semaphore/.gitignore     |  2 --
 testcases/kernel/ipc/semaphore/Makefile       | 30 -------------------
 .../kernel/syscalls/ipc/semop/.gitignore      |  2 ++
 testcases/kernel/syscalls/ipc/semop/Makefile  |  5 +++-
 .../sem01.c => syscalls/ipc/semop/semop04.c}  |  0
 .../sem02.c => syscalls/ipc/semop/semop05.c}  |  0
 8 files changed, 8 insertions(+), 35 deletions(-)
 delete mode 100644 testcases/kernel/ipc/semaphore/.gitignore
 delete mode 100644 testcases/kernel/ipc/semaphore/Makefile
 rename testcases/kernel/{ipc/semaphore/sem01.c => syscalls/ipc/semop/semop04.c} (100%)
 rename testcases/kernel/{ipc/semaphore/sem02.c => syscalls/ipc/semop/semop05.c} (100%)

diff --git a/runtest/ipc b/runtest/ipc
index a2135ed80..db7f7bed5 100644
--- a/runtest/ipc
+++ b/runtest/ipc
@@ -28,5 +28,3 @@ pipeio_8 pipeio -T pipeio_8 -c 5 -s 5000 -i 10 -u -f x80
 # spawns 5 children to write 10 chunks of 5000 bytes to an unnamed pipe
 # using non-blocking I/O
 
-sem01 sem01
-sem02 sem02
diff --git a/runtest/syscalls b/runtest/syscalls
index ae37a1192..54cbc6ece 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1239,6 +1239,8 @@ semget06 semget06
 semop01 semop01
 semop02 semop02
 semop03 semop03
+semop04 semop04
+semop05 semop05
 
 send01 send01
 send02 send02
diff --git a/testcases/kernel/ipc/semaphore/.gitignore b/testcases/kernel/ipc/semaphore/.gitignore
deleted file mode 100644
index 944e5ca1e..000000000
--- a/testcases/kernel/ipc/semaphore/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-/sem01
-/sem02
diff --git a/testcases/kernel/ipc/semaphore/Makefile b/testcases/kernel/ipc/semaphore/Makefile
deleted file mode 100644
index 9ab85745d..000000000
--- a/testcases/kernel/ipc/semaphore/Makefile
+++ /dev/null
@@ -1,30 +0,0 @@
-#
-#    testcases/kernel/ipc/semaphore Makefile.
-#
-#    Copyright (C) 2009, Cisco Systems Inc.
-#
-#    This program is free software; you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License as published by
-#    the Free Software Foundation; either version 2 of the License, or
-#    (at your option) any later version.
-#
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
-#
-#    You should have received a copy of the GNU General Public License along
-#    with this program; if not, write to the Free Software Foundation, Inc.,
-#    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir		?= ../../../..
-
-include $(top_srcdir)/include/mk/testcases.mk
-
-CPPFLAGS 		+= -I../include
-LDLIBS			+= -lpthread
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/ipc/semop/.gitignore b/testcases/kernel/syscalls/ipc/semop/.gitignore
index bb57f08af..cc67b1862 100644
--- a/testcases/kernel/syscalls/ipc/semop/.gitignore
+++ b/testcases/kernel/syscalls/ipc/semop/.gitignore
@@ -1,3 +1,5 @@
 /semop01
 /semop02
 /semop03
+/semop04
+/semop05
diff --git a/testcases/kernel/syscalls/ipc/semop/Makefile b/testcases/kernel/syscalls/ipc/semop/Makefile
index 6b2b26d05..43afffb3f 100644
--- a/testcases/kernel/syscalls/ipc/semop/Makefile
+++ b/testcases/kernel/syscalls/ipc/semop/Makefile
@@ -7,6 +7,9 @@ LTPLIBS = ltpnewipc
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-LTPLDLIBS  = -lltpnewipc
+semop01: LTPLDLIBS  = -lltpnewipc
+semop02: LTPLDLIBS  = -lltpnewipc
+semop03: LTPLDLIBS  = -lltpnewipc
+semop05: LDLIBS    += -lpthread
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/ipc/semaphore/sem01.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem01.c
rename to testcases/kernel/syscalls/ipc/semop/semop04.c
diff --git a/testcases/kernel/ipc/semaphore/sem02.c b/testcases/kernel/syscalls/ipc/semop/semop05.c
similarity index 100%
rename from testcases/kernel/ipc/semaphore/sem02.c
rename to testcases/kernel/syscalls/ipc/semop/semop05.c
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 2/2] semop04:Refactor with new API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ipc/semop/semop04.c | 159 ++++++------------
 1 file changed, 56 insertions(+), 103 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
index 582624d60..589d9b0b4 100644
--- a/testcases/kernel/syscalls/ipc/semop/semop04.c
+++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
@@ -1,36 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (C) 2003-2023 Linux Test Project, Inc.
+ * Author: 2001 Paul Larson <plars@us.ibm.com>
+ * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
  *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-/*
- *  FILE        : sem01.c
- *  DESCRIPTION : Creates a semaphore and two processes.  The processes
- *                each go through a loop where they semdown, delay for a
- *                random amount of time, and semup, so they will almost
- *                always be fighting for control of the semaphore.
- *  HISTORY:
- *    01/15/2001 Paul Larson (plars@us.ibm.com)
- *      -written
- *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
- *    Modified.
- *    - Removed compiler warnings.
- *      added exit to the end of function main()
+/*\
+ * [Description]
  *
+ * Creates a semaphore and two processes.  The processes
+ * each go through a loop where they semdown, delay for a
+ * random amount of time, and semup, so they will almost
+ * always be fighting for control of the semaphore.
  */
 
 #include <unistd.h>
@@ -41,124 +24,94 @@
 #include <sys/wait.h>
 #include <sys/ipc.h>
 #include "lapi/sem.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 
-int verbose = 0;
-int loops = 100;
-int errors = 0;
+static int loops = 100;
+static char *opt_loops_str;
 
-int semup(int semid)
+static void semup(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = 1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semup");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-int semdown(int semid)
+static void semdown(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = -1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semdown");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-void delayloop()
+static void delayloop(void)
 {
 	int delay;
+
 	delay = 1 + ((100.0 * rand()) / RAND_MAX);
-	if (verbose)
-		printf("in delay function for %d microseconds\n", delay);
 	usleep(delay);
 }
 
 void mainloop(int semid)
 {
 	int i;
+
 	for (i = 0; i < loops; i++) {
-		if (semdown(semid)) {
-			printf("semdown failed\n");
-		}
-		if (verbose)
-			printf("sem is down\n");
+		semdown(semid);
 		delayloop();
-		if (semup(semid)) {
-			printf("semup failed\n");
-		}
-		if (verbose)
-			printf("sem is up\n");
+		semup(semid);
 	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int semid, opt;
+	int semid;
 	union semun semunion;
-	extern char *optarg;
 	pid_t pid;
-	int chstat;
-
-	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
-		switch ((char)opt) {
-		case 'l':
-			loops = atoi(optarg);
-			break;
-		case 'v':
-			verbose = 1;
-			break;
-		case 'h':
-		default:
-			printf("Usage: -l loops [-v]\n");
-			exit(1);
-		}
-	}
 
 	/* set up the semaphore */
-	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
-		printf("error in semget()\n");
-		exit(-1);
-	}
+	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
+
 	semunion.val = 1;
-	if (semctl(semid, 0, SETVAL, semunion) == -1) {
-		printf("error in semctl\n");
-	}
 
-	if ((pid = fork()) < 0) {
-		printf("fork error\n");
-		exit(-1);
-	}
+	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
+
+	pid = SAFE_FORK();
+
 	if (pid) {
 		/* parent */
 		srand(pid);
 		mainloop(semid);
-		waitpid(pid, &chstat, 0);
-		if (!WIFEXITED(chstat)) {
-			printf("child exited with status\n");
-			exit(-1);
-		}
-		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
-			printf("error in semctl\n");
-		}
-		if (errors) {
-			printf("FAIL: there were %d errors\n", errors);
-		} else {
-			printf("PASS: error count is 0\n");
-		}
-		exit(errors);
+		tst_reap_children();
+		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
+		tst_res(TPASS, "Semaphore up/down check success");
 	} else {
 		/* child */
 		mainloop(semid);
 	}
-	exit(0);
 }
+
+static void setup(void)
+{
+	if (opt_loops_str)
+		loops = SAFE_STRTOL(opt_loops_str, 1, INT_MAX);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.forks_child = 1,
+	.options = (struct tst_option[]) {
+		{"l:", &opt_loops_str, "Internal loops for semup/down"},
+		{}
+	},
+	.needs_root = 1,
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/2] semop04:Refactor with new API

[Petr Vorel]

Hi Wei

nit: in subject we use space after colon (readability): 
"semop04:Refactor with new API" => "semop04: Refactor with new API"

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/ipc/semop/semop04.c | 159 ++++++------------
>  1 file changed, 56 insertions(+), 103 deletions(-)

> diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
> index 582624d60..589d9b0b4 100644
> --- a/testcases/kernel/syscalls/ipc/semop/semop04.c
> +++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
> @@ -1,36 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> + * Copyright (c) International Business Machines  Corp., 2001
> + * Copyright (C) 2003-2023 Linux Test Project, Inc.
> + * Author: 2001 Paul Larson <plars@us.ibm.com>
> + * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
>   *
nit: Please remove this line with just star ('*'), when it follows */

> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   This program is free software;  you can redistribute it and/or modify
> - *   it under the terms of the GNU General Public License as published by
> - *   the Free Software Foundation; either version 2 of the License, or
> - *   (at your option) any later version.
> - *
> - *   This program is distributed in the hope that it will be useful,
> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - *   the GNU General Public License for more details.
> - *
> - *   You should have received a copy of the GNU General Public License
> - *   along with this program;  if not, write to the Free Software
> - *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */

> -/*
> - *  FILE        : sem01.c
> - *  DESCRIPTION : Creates a semaphore and two processes.  The processes
> - *                each go through a loop where they semdown, delay for a
> - *                random amount of time, and semup, so they will almost
> - *                always be fighting for control of the semaphore.
> - *  HISTORY:
> - *    01/15/2001 Paul Larson (plars@us.ibm.com)
> - *      -written
> - *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
> - *    Modified.
> - *    - Removed compiler warnings.
> - *      added exit to the end of function main()
> +/*\
> + * [Description]
>   *
> + * Creates a semaphore and two processes.  The processes
> + * each go through a loop where they semdown, delay for a
> + * random amount of time, and semup, so they will almost
> + * always be fighting for control of the semaphore.
>   */

>  #include <unistd.h>
> @@ -41,124 +24,94 @@
>  #include <sys/wait.h>
nit: I'd remove this (we no longer use waitpid()).
Also I'd remove <errno.h> (IMHO no errno is used).

>  #include <sys/ipc.h>
>  #include "lapi/sem.h"
> +#include "tst_test.h"
> +#include "tst_safe_sysv_ipc.h"

> -int verbose = 0;
> -int loops = 100;
> -int errors = 0;
> +static int loops = 100;
> +static char *opt_loops_str;
I'm not sure if this worth to have opt for loops.
I'd just define it:
#define LOOPS 1000
and not allow to overwrite (even with 1000 it's fast).

> -int semup(int semid)
> +static void semup(int semid)
>  {
>  	struct sembuf semops;
> +
>  	semops.sem_num = 0;
>  	semops.sem_op = 1;
>  	semops.sem_flg = SEM_UNDO;
> -	if (semop(semid, &semops, 1) == -1) {
> -		perror("semup");
> -		errors++;
> -		return 1;
> -	}
> -	return 0;
> +
> +	SAFE_SEMOP(semid, &semops, 1);
>  }

> -int semdown(int semid)
> +static void semdown(int semid)
>  {
>  	struct sembuf semops;
> +
>  	semops.sem_num = 0;
>  	semops.sem_op = -1;
>  	semops.sem_flg = SEM_UNDO;
> -	if (semop(semid, &semops, 1) == -1) {
> -		perror("semdown");
> -		errors++;
> -		return 1;
> -	}
> -	return 0;
> +
> +	SAFE_SEMOP(semid, &semops, 1);
>  }

> -void delayloop()
> +static void delayloop(void)
>  {
>  	int delay;
> +
>  	delay = 1 + ((100.0 * rand()) / RAND_MAX);
> -	if (verbose)
> -		printf("in delay function for %d microseconds\n", delay);
>  	usleep(delay);
>  }

>  void mainloop(int semid)
This should also be static.

>  {
>  	int i;
...
> +	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
> +
> +	pid = SAFE_FORK();
> +
>  	if (pid) {
>  		/* parent */
Please remove this obvious comment.
>  		srand(pid);
>  		mainloop(semid);
...
> +		tst_reap_children();
> +		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
> +		tst_res(TPASS, "Semaphore up/down check success");
>  	} else {
>  		/* child */
And this one.
>  		mainloop(semid);
>  	}
> -	exit(0);
>  }
> +
> +static void setup(void)
> +{
> +	if (opt_loops_str)
> +		loops = SAFE_STRTOL(opt_loops_str, 1, INT_MAX);
> +}
This is to be removed.

> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.forks_child = 1,
> +	.options = (struct tst_option[]) {
> +		{"l:", &opt_loops_str, "Internal loops for semup/down"},
> +		{}
This is going to be removed.
> +	},
> +	.needs_root = 1,
Do we really need root? I guess no.

Kind regards,
Petr

> +};

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [v5,2/2] semop04: Refactor with new API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ipc/semop/semop04.c | 155 +++++-------------
 1 file changed, 45 insertions(+), 110 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
index 582624d60..dee09d9fd 100644
--- a/testcases/kernel/syscalls/ipc/semop/semop04.c
+++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
@@ -1,164 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (C) 2003-2023 Linux Test Project, Inc.
+ * Author: 2001 Paul Larson <plars@us.ibm.com>
+ * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
  */
 
-/*
- *  FILE        : sem01.c
- *  DESCRIPTION : Creates a semaphore and two processes.  The processes
- *                each go through a loop where they semdown, delay for a
- *                random amount of time, and semup, so they will almost
- *                always be fighting for control of the semaphore.
- *  HISTORY:
- *    01/15/2001 Paul Larson (plars@us.ibm.com)
- *      -written
- *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
- *    Modified.
- *    - Removed compiler warnings.
- *      added exit to the end of function main()
+/*\
+ * [Description]
  *
+ * Creates a semaphore and two processes.  The processes
+ * each go through a loop where they semdown, delay for a
+ * random amount of time, and semup, so they will almost
+ * always be fighting for control of the semaphore.
  */
 
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <errno.h>
 #include <sys/types.h>
-#include <sys/wait.h>
 #include <sys/ipc.h>
 #include "lapi/sem.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 
-int verbose = 0;
-int loops = 100;
-int errors = 0;
+#define LOOPS 1000
 
-int semup(int semid)
+static void semup(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = 1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semup");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-int semdown(int semid)
+static void semdown(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = -1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semdown");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-void delayloop()
+static void delayloop(void)
 {
 	int delay;
+
 	delay = 1 + ((100.0 * rand()) / RAND_MAX);
-	if (verbose)
-		printf("in delay function for %d microseconds\n", delay);
 	usleep(delay);
 }
 
-void mainloop(int semid)
+static void mainloop(int semid)
 {
 	int i;
-	for (i = 0; i < loops; i++) {
-		if (semdown(semid)) {
-			printf("semdown failed\n");
-		}
-		if (verbose)
-			printf("sem is down\n");
+
+	for (i = 0; i < LOOPS; i++) {
+		semdown(semid);
 		delayloop();
-		if (semup(semid)) {
-			printf("semup failed\n");
-		}
-		if (verbose)
-			printf("sem is up\n");
+		semup(semid);
 	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int semid, opt;
+	int semid;
 	union semun semunion;
-	extern char *optarg;
 	pid_t pid;
-	int chstat;
-
-	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
-		switch ((char)opt) {
-		case 'l':
-			loops = atoi(optarg);
-			break;
-		case 'v':
-			verbose = 1;
-			break;
-		case 'h':
-		default:
-			printf("Usage: -l loops [-v]\n");
-			exit(1);
-		}
-	}
 
 	/* set up the semaphore */
-	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
-		printf("error in semget()\n");
-		exit(-1);
-	}
+	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
+
 	semunion.val = 1;
-	if (semctl(semid, 0, SETVAL, semunion) == -1) {
-		printf("error in semctl\n");
-	}
 
-	if ((pid = fork()) < 0) {
-		printf("fork error\n");
-		exit(-1);
-	}
+	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
+
+	pid = SAFE_FORK();
+
 	if (pid) {
-		/* parent */
 		srand(pid);
 		mainloop(semid);
-		waitpid(pid, &chstat, 0);
-		if (!WIFEXITED(chstat)) {
-			printf("child exited with status\n");
-			exit(-1);
-		}
-		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
-			printf("error in semctl\n");
-		}
-		if (errors) {
-			printf("FAIL: there were %d errors\n", errors);
-		} else {
-			printf("PASS: error count is 0\n");
-		}
-		exit(errors);
+		tst_reap_children();
+		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
+		tst_res(TPASS, "Semaphore up/down check success");
 	} else {
-		/* child */
 		mainloop(semid);
 	}
-	exit(0);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [v5,2/2] semop04: Refactor with new API

[Petr Vorel]

Hi Wei,

> -void delayloop()
> +static void delayloop(void)
>  {
>  	int delay;
> +
>  	delay = 1 + ((100.0 * rand()) / RAND_MAX);
> -	if (verbose)
> -		printf("in delay function for %d microseconds\n", delay);
>  	usleep(delay);
>  }

...
> +	for (i = 0; i < LOOPS; i++) {
> +		semdown(semid);
>  		delayloop();
Why not just:

	usleep(1 + ((100.0 * rand()) / RAND_MAX));

and delete whole delayloop() function?

> -		if (semup(semid)) {
> -			printf("semup failed\n");
> -		}
> -		if (verbose)
> -			printf("sem is up\n");
> +		semup(semid);
>  	}

...
>  	/* set up the semaphore */
> -	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
> -		printf("error in semget()\n");
> -		exit(-1);
> -	}
> +	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
> +
>  	semunion.val = 1;
> -	if (semctl(semid, 0, SETVAL, semunion) == -1) {
> -		printf("error in semctl\n");
> -	}

> -	if ((pid = fork()) < 0) {
> -		printf("fork error\n");
> -		exit(-1);
> -	}
> +	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
> +
> +	pid = SAFE_FORK();
> +
>  	if (pid) {
> -		/* parent */
>  		srand(pid);
>  		mainloop(semid);
> -		waitpid(pid, &chstat, 0);
> -		if (!WIFEXITED(chstat)) {
> -			printf("child exited with status\n");
> -			exit(-1);
> -		}
> -		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
> -			printf("error in semctl\n");
> -		}
> -		if (errors) {
> -			printf("FAIL: there were %d errors\n", errors);
> -		} else {
> -			printf("PASS: error count is 0\n");
> -		}
> -		exit(errors);
> +		tst_reap_children();
> +		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
> +		tst_res(TPASS, "Semaphore up/down check success");

Maybe here use plain semctl() verified with TST_EXP_POSITIVE() from
tst_test_macros.h? This way we'd get TFAIL on failure instead of TPASS.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [v5,2/2] semop04: Refactor with new API

[Richard Palethorpe]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

> Hi Wei,
>
>> -void delayloop()
>> +static void delayloop(void)
>>  {
>>  	int delay;
>> +
>>  	delay = 1 + ((100.0 * rand()) / RAND_MAX);
>> -	if (verbose)
>> -		printf("in delay function for %d microseconds\n", delay);
>>  	usleep(delay);
>>  }
>
> ...
>> +	for (i = 0; i < LOOPS; i++) {
>> +		semdown(semid);
>>  		delayloop();
> Why not just:
>
> 	usleep(1 + ((100.0 * rand()) / RAND_MAX));
>
> and delete whole delayloop() function?
>
>> -		if (semup(semid)) {
>> -			printf("semup failed\n");
>> -		}
>> -		if (verbose)
>> -			printf("sem is up\n");
>> +		semup(semid);
>>  	}
>
> ...
>>  	/* set up the semaphore */
>> -	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
>> -		printf("error in semget()\n");
>> -		exit(-1);
>> -	}
>> +	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
>> +
>>  	semunion.val = 1;
>> -	if (semctl(semid, 0, SETVAL, semunion) == -1) {
>> -		printf("error in semctl\n");
>> -	}
>
>> -	if ((pid = fork()) < 0) {
>> -		printf("fork error\n");
>> -		exit(-1);
>> -	}
>> +	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
>> +
>> +	pid = SAFE_FORK();
>> +
>>  	if (pid) {
>> -		/* parent */
>>  		srand(pid);
>>  		mainloop(semid);
>> -		waitpid(pid, &chstat, 0);
>> -		if (!WIFEXITED(chstat)) {
>> -			printf("child exited with status\n");
>> -			exit(-1);
>> -		}
>> -		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
>> -			printf("error in semctl\n");
>> -		}
>> -		if (errors) {
>> -			printf("FAIL: there were %d errors\n", errors);
>> -		} else {
>> -			printf("PASS: error count is 0\n");
>> -		}
>> -		exit(errors);
>> +		tst_reap_children();
>> +		SAFE_SEMCTL(semid, 0, IPC_RMID, semunion);
>> +		tst_res(TPASS, "Semaphore up/down check success");
>
> Maybe here use plain semctl() verified with TST_EXP_POSITIVE() from
> tst_test_macros.h? This way we'd get TFAIL on failure instead of TPASS.
>
> Kind regards,
> Petr

Setting to changes requested in patchwork.


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [v6,2/2] semop04: Refactor with new API

[Wei Gao via ltp]

---
 testcases/kernel/syscalls/ipc/semop/semop04.c | 158 +++++-------------
 1 file changed, 42 insertions(+), 116 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
index 582624d60..96f4b8fb8 100644
--- a/testcases/kernel/syscalls/ipc/semop/semop04.c
+++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
@@ -1,164 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (C) 2003-2023 Linux Test Project, Inc.
+ * Author: 2001 Paul Larson <plars@us.ibm.com>
+ * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
  */
 
-/*
- *  FILE        : sem01.c
- *  DESCRIPTION : Creates a semaphore and two processes.  The processes
- *                each go through a loop where they semdown, delay for a
- *                random amount of time, and semup, so they will almost
- *                always be fighting for control of the semaphore.
- *  HISTORY:
- *    01/15/2001 Paul Larson (plars@us.ibm.com)
- *      -written
- *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
- *    Modified.
- *    - Removed compiler warnings.
- *      added exit to the end of function main()
+/*\
+ * [Description]
  *
+ * Creates a semaphore and two processes.  The processes
+ * each go through a loop where they semdown, delay for a
+ * random amount of time, and semup, so they will almost
+ * always be fighting for control of the semaphore.
  */
 
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <errno.h>
 #include <sys/types.h>
-#include <sys/wait.h>
 #include <sys/ipc.h>
 #include "lapi/sem.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 
-int verbose = 0;
-int loops = 100;
-int errors = 0;
+#define LOOPS 1000
 
-int semup(int semid)
+static void semup(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = 1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semup");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-int semdown(int semid)
+static void semdown(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = -1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semdown");
-		errors++;
-		return 1;
-	}
-	return 0;
-}
 
-void delayloop()
-{
-	int delay;
-	delay = 1 + ((100.0 * rand()) / RAND_MAX);
-	if (verbose)
-		printf("in delay function for %d microseconds\n", delay);
-	usleep(delay);
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-void mainloop(int semid)
+static void mainloop(int semid)
 {
 	int i;
-	for (i = 0; i < loops; i++) {
-		if (semdown(semid)) {
-			printf("semdown failed\n");
-		}
-		if (verbose)
-			printf("sem is down\n");
-		delayloop();
-		if (semup(semid)) {
-			printf("semup failed\n");
-		}
-		if (verbose)
-			printf("sem is up\n");
+
+	for (i = 0; i < LOOPS; i++) {
+		semdown(semid);
+		usleep(1 + ((100.0 * rand()) / RAND_MAX));
+		semup(semid);
 	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int semid, opt;
+	int semid;
 	union semun semunion;
-	extern char *optarg;
 	pid_t pid;
-	int chstat;
-
-	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
-		switch ((char)opt) {
-		case 'l':
-			loops = atoi(optarg);
-			break;
-		case 'v':
-			verbose = 1;
-			break;
-		case 'h':
-		default:
-			printf("Usage: -l loops [-v]\n");
-			exit(1);
-		}
-	}
 
 	/* set up the semaphore */
-	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
-		printf("error in semget()\n");
-		exit(-1);
-	}
+	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
+
 	semunion.val = 1;
-	if (semctl(semid, 0, SETVAL, semunion) == -1) {
-		printf("error in semctl\n");
-	}
 
-	if ((pid = fork()) < 0) {
-		printf("fork error\n");
-		exit(-1);
-	}
+	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
+
+	pid = SAFE_FORK();
+
 	if (pid) {
-		/* parent */
 		srand(pid);
 		mainloop(semid);
-		waitpid(pid, &chstat, 0);
-		if (!WIFEXITED(chstat)) {
-			printf("child exited with status\n");
-			exit(-1);
-		}
-		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
-			printf("error in semctl\n");
-		}
-		if (errors) {
-			printf("FAIL: there were %d errors\n", errors);
-		} else {
-			printf("PASS: error count is 0\n");
-		}
-		exit(errors);
+		tst_reap_children();
+		TST_EXP_POSITIVE(semctl(semid, 0, IPC_RMID, semunion));
 	} else {
-		/* child */
 		mainloop(semid);
 	}
-	exit(0);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [v7,2/2] semop04: Refactor with new API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ipc/semop/semop04.c | 158 +++++-------------
 1 file changed, 42 insertions(+), 116 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
index 582624d60..96f4b8fb8 100644
--- a/testcases/kernel/syscalls/ipc/semop/semop04.c
+++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
@@ -1,164 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (C) 2003-2023 Linux Test Project, Inc.
+ * Author: 2001 Paul Larson <plars@us.ibm.com>
+ * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
  */
 
-/*
- *  FILE        : sem01.c
- *  DESCRIPTION : Creates a semaphore and two processes.  The processes
- *                each go through a loop where they semdown, delay for a
- *                random amount of time, and semup, so they will almost
- *                always be fighting for control of the semaphore.
- *  HISTORY:
- *    01/15/2001 Paul Larson (plars@us.ibm.com)
- *      -written
- *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
- *    Modified.
- *    - Removed compiler warnings.
- *      added exit to the end of function main()
+/*\
+ * [Description]
  *
+ * Creates a semaphore and two processes.  The processes
+ * each go through a loop where they semdown, delay for a
+ * random amount of time, and semup, so they will almost
+ * always be fighting for control of the semaphore.
  */
 
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <errno.h>
 #include <sys/types.h>
-#include <sys/wait.h>
 #include <sys/ipc.h>
 #include "lapi/sem.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 
-int verbose = 0;
-int loops = 100;
-int errors = 0;
+#define LOOPS 1000
 
-int semup(int semid)
+static void semup(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = 1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semup");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-int semdown(int semid)
+static void semdown(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = -1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semdown");
-		errors++;
-		return 1;
-	}
-	return 0;
-}
 
-void delayloop()
-{
-	int delay;
-	delay = 1 + ((100.0 * rand()) / RAND_MAX);
-	if (verbose)
-		printf("in delay function for %d microseconds\n", delay);
-	usleep(delay);
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-void mainloop(int semid)
+static void mainloop(int semid)
 {
 	int i;
-	for (i = 0; i < loops; i++) {
-		if (semdown(semid)) {
-			printf("semdown failed\n");
-		}
-		if (verbose)
-			printf("sem is down\n");
-		delayloop();
-		if (semup(semid)) {
-			printf("semup failed\n");
-		}
-		if (verbose)
-			printf("sem is up\n");
+
+	for (i = 0; i < LOOPS; i++) {
+		semdown(semid);
+		usleep(1 + ((100.0 * rand()) / RAND_MAX));
+		semup(semid);
 	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int semid, opt;
+	int semid;
 	union semun semunion;
-	extern char *optarg;
 	pid_t pid;
-	int chstat;
-
-	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
-		switch ((char)opt) {
-		case 'l':
-			loops = atoi(optarg);
-			break;
-		case 'v':
-			verbose = 1;
-			break;
-		case 'h':
-		default:
-			printf("Usage: -l loops [-v]\n");
-			exit(1);
-		}
-	}
 
 	/* set up the semaphore */
-	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
-		printf("error in semget()\n");
-		exit(-1);
-	}
+	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
+
 	semunion.val = 1;
-	if (semctl(semid, 0, SETVAL, semunion) == -1) {
-		printf("error in semctl\n");
-	}
 
-	if ((pid = fork()) < 0) {
-		printf("fork error\n");
-		exit(-1);
-	}
+	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
+
+	pid = SAFE_FORK();
+
 	if (pid) {
-		/* parent */
 		srand(pid);
 		mainloop(semid);
-		waitpid(pid, &chstat, 0);
-		if (!WIFEXITED(chstat)) {
-			printf("child exited with status\n");
-			exit(-1);
-		}
-		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
-			printf("error in semctl\n");
-		}
-		if (errors) {
-			printf("FAIL: there were %d errors\n", errors);
-		} else {
-			printf("PASS: error count is 0\n");
-		}
-		exit(errors);
+		tst_reap_children();
+		TST_EXP_POSITIVE(semctl(semid, 0, IPC_RMID, semunion));
 	} else {
-		/* child */
 		mainloop(semid);
 	}
-	exit(0);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [v7,2/2] semop04: Refactor with new API

[Andrea Cervesato via ltp]

Hi!

On 8/29/23 13:18, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>   testcases/kernel/syscalls/ipc/semop/semop04.c | 158 +++++-------------
>   1 file changed, 42 insertions(+), 116 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
> index 582624d60..96f4b8fb8 100644
> --- a/testcases/kernel/syscalls/ipc/semop/semop04.c
> +++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
> @@ -1,164 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - *
> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   This program is free software;  you can redistribute it and/or modify
> - *   it under the terms of the GNU General Public License as published by
> - *   the Free Software Foundation; either version 2 of the License, or
> - *   (at your option) any later version.
> - *
> - *   This program is distributed in the hope that it will be useful,
> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - *   the GNU General Public License for more details.
> - *
> - *   You should have received a copy of the GNU General Public License
> - *   along with this program;  if not, write to the Free Software
> - *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * Copyright (c) International Business Machines  Corp., 2001
> + * Copyright (C) 2003-2023 Linux Test Project, Inc.
> + * Author: 2001 Paul Larson <plars@us.ibm.com>
> + * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
>    */
>   
> -/*
> - *  FILE        : sem01.c
> - *  DESCRIPTION : Creates a semaphore and two processes.  The processes
> - *                each go through a loop where they semdown, delay for a
> - *                random amount of time, and semup, so they will almost
> - *                always be fighting for control of the semaphore.
> - *  HISTORY:
> - *    01/15/2001 Paul Larson (plars@us.ibm.com)
> - *      -written
> - *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
> - *    Modified.
> - *    - Removed compiler warnings.
> - *      added exit to the end of function main()
> +/*\
> + * [Description]
>    *
> + * Creates a semaphore and two processes.  The processes
> + * each go through a loop where they semdown, delay for a
> + * random amount of time, and semup, so they will almost
> + * always be fighting for control of the semaphore.
>    */
>   
>   #include <unistd.h>
>   #include <stdlib.h>
>   #include <stdio.h>
> -#include <errno.h>
>   #include <sys/types.h>
> -#include <sys/wait.h>
>   #include <sys/ipc.h>
>   #include "lapi/sem.h"
> +#include "tst_test.h"
> +#include "tst_safe_sysv_ipc.h"
>   
> -int verbose = 0;
> -int loops = 100;
> -int errors = 0;
> +#define LOOPS 1000
>   
> -int semup(int semid)
> +static void semup(int semid)
>   {
>   	struct sembuf semops;
> +
>   	semops.sem_num = 0;
>   	semops.sem_op = 1;
>   	semops.sem_flg = SEM_UNDO;
> -	if (semop(semid, &semops, 1) == -1) {
> -		perror("semup");
> -		errors++;
> -		return 1;
> -	}
> -	return 0;
> +
> +	SAFE_SEMOP(semid, &semops, 1);
>   }
>   
> -int semdown(int semid)
> +static void semdown(int semid)
>   {
>   	struct sembuf semops;
> +
>   	semops.sem_num = 0;
>   	semops.sem_op = -1;
>   	semops.sem_flg = SEM_UNDO;
> -	if (semop(semid, &semops, 1) == -1) {
> -		perror("semdown");
> -		errors++;
> -		return 1;
> -	}
> -	return 0;
> -}
>   
> -void delayloop()
> -{
> -	int delay;
> -	delay = 1 + ((100.0 * rand()) / RAND_MAX);
> -	if (verbose)
> -		printf("in delay function for %d microseconds\n", delay);
> -	usleep(delay);
> +	SAFE_SEMOP(semid, &semops, 1);
>   }
>   
> -void mainloop(int semid)
> +static void mainloop(int semid)
>   {
>   	int i;
> -	for (i = 0; i < loops; i++) {
> -		if (semdown(semid)) {
> -			printf("semdown failed\n");
> -		}
> -		if (verbose)
> -			printf("sem is down\n");
> -		delayloop();
> -		if (semup(semid)) {
> -			printf("semup failed\n");
> -		}
> -		if (verbose)
> -			printf("sem is up\n");
> +
> +	for (i = 0; i < LOOPS; i++) {
> +		semdown(semid);
> +		usleep(1 + ((100.0 * rand()) / RAND_MAX));
> +		semup(semid);
>   	}
>   }
>   
> -int main(int argc, char *argv[])
> +static void run(void)
>   {
> -	int semid, opt;
> +	int semid;
>   	union semun semunion;
> -	extern char *optarg;
>   	pid_t pid;
> -	int chstat;
> -
> -	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
> -		switch ((char)opt) {
> -		case 'l':
> -			loops = atoi(optarg);
> -			break;
> -		case 'v':
> -			verbose = 1;
> -			break;
> -		case 'h':
> -		default:
> -			printf("Usage: -l loops [-v]\n");
> -			exit(1);
> -		}
> -	}
>   
>   	/* set up the semaphore */
> -	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
> -		printf("error in semget()\n");
> -		exit(-1);
> -	}
> +	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
> +
>   	semunion.val = 1;
> -	if (semctl(semid, 0, SETVAL, semunion) == -1) {
> -		printf("error in semctl\n");
> -	}
>   
> -	if ((pid = fork()) < 0) {
> -		printf("fork error\n");
> -		exit(-1);
> -	}
> +	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
> +
> +	pid = SAFE_FORK();
> +
>   	if (pid) {
> -		/* parent */
>   		srand(pid);
I'm wondering if this is an error from the previous test as well. We are 
using mainloop() function that is using rand() inside both child and 
parent. So srand() should be called before parent and child, in order to 
have random seed initialized for both of them. I would also use 
srand(time(0)).
>   		mainloop(semid);
> -		waitpid(pid, &chstat, 0);
> -		if (!WIFEXITED(chstat)) {
> -			printf("child exited with status\n");
> -			exit(-1);
> -		}
> -		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
> -			printf("error in semctl\n");
> -		}
> -		if (errors) {
> -			printf("FAIL: there were %d errors\n", errors);
> -		} else {
> -			printf("PASS: error count is 0\n");
> -		}
> -		exit(errors);
> +		tst_reap_children();
> +		TST_EXP_POSITIVE(semctl(semid, 0, IPC_RMID, semunion));
>   	} else {
> -		/* child */
>   		mainloop(semid);
>   	}
> -	exit(0);
>   }
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.forks_child = 1,
> +};

Thank you,
Andrea Cervesato


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [v7,2/2] semop04: Refactor with new API

[Richard Palethorpe]

Hello,

Andrea Cervesato via ltp <ltp@lists.linux.it> writes:

> Hi!
>
> On 8/29/23 13:18, Wei Gao via ltp wrote:
>> Signed-off-by: Wei Gao <wegao@suse.com>
>> ---
>>   testcases/kernel/syscalls/ipc/semop/semop04.c | 158 +++++-------------
>>   1 file changed, 42 insertions(+), 116 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
>> index 582624d60..96f4b8fb8 100644
>> --- a/testcases/kernel/syscalls/ipc/semop/semop04.c
>> +++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
>> @@ -1,164 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>> - *
>> - *   Copyright (c) International Business Machines  Corp., 2001
>> - *
>> - *   This program is free software;  you can redistribute it and/or modify
>> - *   it under the terms of the GNU General Public License as published by
>> - *   the Free Software Foundation; either version 2 of the License, or
>> - *   (at your option) any later version.
>> - *
>> - *   This program is distributed in the hope that it will be useful,
>> - *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
>> - *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
>> - *   the GNU General Public License for more details.
>> - *
>> - *   You should have received a copy of the GNU General Public License
>> - *   along with this program;  if not, write to the Free Software
>> - *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + * Copyright (c) International Business Machines  Corp., 2001
>> + * Copyright (C) 2003-2023 Linux Test Project, Inc.
>> + * Author: 2001 Paul Larson <plars@us.ibm.com>
>> + * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
>>    */
>>   -/*
>> - *  FILE        : sem01.c
>> - *  DESCRIPTION : Creates a semaphore and two processes.  The processes
>> - *                each go through a loop where they semdown, delay for a
>> - *                random amount of time, and semup, so they will almost
>> - *                always be fighting for control of the semaphore.
>> - *  HISTORY:
>> - *    01/15/2001 Paul Larson (plars@us.ibm.com)
>> - *      -written
>> - *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
>> - *    Modified.
>> - *    - Removed compiler warnings.
>> - *      added exit to the end of function main()
>> +/*\
>> + * [Description]
>>    *
>> + * Creates a semaphore and two processes.  The processes
>> + * each go through a loop where they semdown, delay for a
>> + * random amount of time, and semup, so they will almost
>> + * always be fighting for control of the semaphore.
>>    */
>>     #include <unistd.h>
>>   #include <stdlib.h>
>>   #include <stdio.h>
>> -#include <errno.h>
>>   #include <sys/types.h>
>> -#include <sys/wait.h>
>>   #include <sys/ipc.h>
>>   #include "lapi/sem.h"
>> +#include "tst_test.h"
>> +#include "tst_safe_sysv_ipc.h"
>>   -int verbose = 0;
>> -int loops = 100;
>> -int errors = 0;
>> +#define LOOPS 1000
>>   -int semup(int semid)
>> +static void semup(int semid)
>>   {
>>   	struct sembuf semops;
>> +
>>   	semops.sem_num = 0;
>>   	semops.sem_op = 1;
>>   	semops.sem_flg = SEM_UNDO;
>> -	if (semop(semid, &semops, 1) == -1) {
>> -		perror("semup");
>> -		errors++;
>> -		return 1;
>> -	}
>> -	return 0;
>> +
>> +	SAFE_SEMOP(semid, &semops, 1);
>>   }
>>   -int semdown(int semid)
>> +static void semdown(int semid)
>>   {
>>   	struct sembuf semops;
>> +
>>   	semops.sem_num = 0;
>>   	semops.sem_op = -1;
>>   	semops.sem_flg = SEM_UNDO;
>> -	if (semop(semid, &semops, 1) == -1) {
>> -		perror("semdown");
>> -		errors++;
>> -		return 1;
>> -	}
>> -	return 0;
>> -}
>>   -void delayloop()
>> -{
>> -	int delay;
>> -	delay = 1 + ((100.0 * rand()) / RAND_MAX);
>> -	if (verbose)
>> -		printf("in delay function for %d microseconds\n", delay);
>> -	usleep(delay);
>> +	SAFE_SEMOP(semid, &semops, 1);
>>   }
>>   -void mainloop(int semid)
>> +static void mainloop(int semid)
>>   {
>>   	int i;
>> -	for (i = 0; i < loops; i++) {
>> -		if (semdown(semid)) {
>> -			printf("semdown failed\n");
>> -		}
>> -		if (verbose)
>> -			printf("sem is down\n");
>> -		delayloop();
>> -		if (semup(semid)) {
>> -			printf("semup failed\n");
>> -		}
>> -		if (verbose)
>> -			printf("sem is up\n");
>> +
>> +	for (i = 0; i < LOOPS; i++) {
>> +		semdown(semid);
>> +		usleep(1 + ((100.0 * rand()) / RAND_MAX));
>> +		semup(semid);
>>   	}
>>   }
>>   -int main(int argc, char *argv[])
>> +static void run(void)
>>   {
>> -	int semid, opt;
>> +	int semid;
>>   	union semun semunion;
>> -	extern char *optarg;
>>   	pid_t pid;
>> -	int chstat;
>> -
>> -	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
>> -		switch ((char)opt) {
>> -		case 'l':
>> -			loops = atoi(optarg);
>> -			break;
>> -		case 'v':
>> -			verbose = 1;
>> -			break;
>> -		case 'h':
>> -		default:
>> -			printf("Usage: -l loops [-v]\n");
>> -			exit(1);
>> -		}
>> -	}
>>     	/* set up the semaphore */
>> -	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
>> -		printf("error in semget()\n");
>> -		exit(-1);
>> -	}
>> +	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
>> +
>>   	semunion.val = 1;
>> -	if (semctl(semid, 0, SETVAL, semunion) == -1) {
>> -		printf("error in semctl\n");
>> -	}
>>   -	if ((pid = fork()) < 0) {
>> -		printf("fork error\n");
>> -		exit(-1);
>> -	}
>> +	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
>> +
>> +	pid = SAFE_FORK();
>> +
>>   	if (pid) {
>> -		/* parent */
>>   		srand(pid);
> I'm wondering if this is an error from the previous test as well. We
> are using mainloop() function that is using rand() inside both child
> and parent. So srand() should be called before parent and child, in
> order to have random seed initialized for both of them. I would also
> use srand(time(0)).

Perhaps and also the value passed to srand should be printed for
reproducing failures. (or just use the same value every time).

Setting to changes requested.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v8] [v7,2/2] semop04: Refactor with new API

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/ipc/semop/semop04.c | 163 +++++-------------
 1 file changed, 46 insertions(+), 117 deletions(-)

diff --git a/testcases/kernel/syscalls/ipc/semop/semop04.c b/testcases/kernel/syscalls/ipc/semop/semop04.c
index 582624d60..1f49e7740 100644
--- a/testcases/kernel/syscalls/ipc/semop/semop04.c
+++ b/testcases/kernel/syscalls/ipc/semop/semop04.c
@@ -1,164 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
- *
- *   Copyright (c) International Business Machines  Corp., 2001
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Copyright (c) International Business Machines  Corp., 2001
+ * Copyright (C) 2003-2023 Linux Test Project, Inc.
+ * Author: 2001 Paul Larson <plars@us.ibm.com>
+ * Modified: 2001 Manoj Iyer <manjo@ausin.ibm.com>
  */
 
-/*
- *  FILE        : sem01.c
- *  DESCRIPTION : Creates a semaphore and two processes.  The processes
- *                each go through a loop where they semdown, delay for a
- *                random amount of time, and semup, so they will almost
- *                always be fighting for control of the semaphore.
- *  HISTORY:
- *    01/15/2001 Paul Larson (plars@us.ibm.com)
- *      -written
- *    11/09/2001 Manoj Iyer (manjo@ausin.ibm.com)
- *    Modified.
- *    - Removed compiler warnings.
- *      added exit to the end of function main()
+/*\
+ * [Description]
  *
+ * Creates a semaphore and two processes.  The processes
+ * each go through a loop where they semdown, delay for a
+ * random amount of time, and semup, so they will almost
+ * always be fighting for control of the semaphore.
  */
 
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <errno.h>
 #include <sys/types.h>
-#include <sys/wait.h>
 #include <sys/ipc.h>
 #include "lapi/sem.h"
+#include "tst_test.h"
+#include "tst_safe_sysv_ipc.h"
 
-int verbose = 0;
-int loops = 100;
-int errors = 0;
+#define LOOPS 1000
+#define SEED 123
 
-int semup(int semid)
+static void semup(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = 1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semup");
-		errors++;
-		return 1;
-	}
-	return 0;
+
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-int semdown(int semid)
+static void semdown(int semid)
 {
 	struct sembuf semops;
+
 	semops.sem_num = 0;
 	semops.sem_op = -1;
 	semops.sem_flg = SEM_UNDO;
-	if (semop(semid, &semops, 1) == -1) {
-		perror("semdown");
-		errors++;
-		return 1;
-	}
-	return 0;
-}
 
-void delayloop()
-{
-	int delay;
-	delay = 1 + ((100.0 * rand()) / RAND_MAX);
-	if (verbose)
-		printf("in delay function for %d microseconds\n", delay);
-	usleep(delay);
+	SAFE_SEMOP(semid, &semops, 1);
 }
 
-void mainloop(int semid)
+static void mainloop(int semid)
 {
 	int i;
-	for (i = 0; i < loops; i++) {
-		if (semdown(semid)) {
-			printf("semdown failed\n");
-		}
-		if (verbose)
-			printf("sem is down\n");
-		delayloop();
-		if (semup(semid)) {
-			printf("semup failed\n");
-		}
-		if (verbose)
-			printf("sem is up\n");
+
+	for (i = 0; i < LOOPS; i++) {
+		semdown(semid);
+		usleep(1 + ((100.0 * rand()) / RAND_MAX));
+		semup(semid);
 	}
 }
 
-int main(int argc, char *argv[])
+static void run(void)
 {
-	int semid, opt;
+	int semid;
 	union semun semunion;
-	extern char *optarg;
 	pid_t pid;
-	int chstat;
-
-	while ((opt = getopt(argc, argv, "l:vh")) != EOF) {
-		switch ((char)opt) {
-		case 'l':
-			loops = atoi(optarg);
-			break;
-		case 'v':
-			verbose = 1;
-			break;
-		case 'h':
-		default:
-			printf("Usage: -l loops [-v]\n");
-			exit(1);
-		}
-	}
 
 	/* set up the semaphore */
-	if ((semid = semget((key_t) 9142, 1, 0666 | IPC_CREAT)) < 0) {
-		printf("error in semget()\n");
-		exit(-1);
-	}
+	semid = SAFE_SEMGET((key_t) 9142, 1, 0666 | IPC_CREAT);
+
 	semunion.val = 1;
-	if (semctl(semid, 0, SETVAL, semunion) == -1) {
-		printf("error in semctl\n");
-	}
 
-	if ((pid = fork()) < 0) {
-		printf("fork error\n");
-		exit(-1);
-	}
+	SAFE_SEMCTL(semid, 0, SETVAL, semunion);
+
+	tst_res(TINFO, "srand seed is %d", SEED);
+	srand(SEED);
+
+	pid = SAFE_FORK();
+
 	if (pid) {
-		/* parent */
-		srand(pid);
 		mainloop(semid);
-		waitpid(pid, &chstat, 0);
-		if (!WIFEXITED(chstat)) {
-			printf("child exited with status\n");
-			exit(-1);
-		}
-		if (semctl(semid, 0, IPC_RMID, semunion) == -1) {
-			printf("error in semctl\n");
-		}
-		if (errors) {
-			printf("FAIL: there were %d errors\n", errors);
-		} else {
-			printf("PASS: error count is 0\n");
-		}
-		exit(errors);
+		tst_reap_children();
+		TST_EXP_POSITIVE(semctl(semid, 0, IPC_RMID, semunion));
 	} else {
-		/* child */
 		mainloop(semid);
 	}
-	exit(0);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.forks_child = 1,
+};
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp