diff mbox

[12/17] Add SYSV semaphore test

Message ID 1482847286-29933-13-git-send-email-adhemerval.zanella@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella Netto Dec. 27, 2016, 2:01 p.m. UTC 
This patch adds a simple SYSV semaphore test to check for correct
argument passing on kernel.  The idea is neither to be an extensive
testing nor to check for any specific Linux test.

	* sysvipc/Makefile (tests): Add test-sysvsem.
	* sysvipc/test-sysvsem.c: New file.
---
 ChangeLog              |   3 ++
 sysvipc/Makefile       |   2 +-
 sysvipc/test-sysvsem.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 sysvipc/test-sysvsem.c

-- 
2.7.4

Comments

Florian Weimer Dec. 31, 2016, 9:07 a.m. UTC | #1 
On 12/30/2016 09:43 PM, Tulio Magno Quites Machado Filho wrote:
> +/* Confirm if sys/sem.h defines semun.  */

> +#ifdef _SEM_SEMUN_UNDEFINED

> +union semun

> +{

> +  int val;

> +  struct semid_ds *buf;

> +    unsigned short int *array;

> +  struct seminfo *__buf;

> +};

> +#endif


Sorry, I don't understand the comment.  Why is this definition not 
provided by the installed headers if IPC_STAT needs it?

Thanks,
Florian
Zack Weinberg Dec. 31, 2016, 4:04 p.m. UTC | #2 
On Sat, Dec 31, 2016 at 1:07 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 12/30/2016 09:43 PM, Tulio Magno Quites Machado Filho wrote:

>>

>> +/* Confirm if sys/sem.h defines semun.  */

>> +#ifdef _SEM_SEMUN_UNDEFINED

>> +union semun

>> +{

>> +  int val;

>> +  struct semid_ds *buf;

>> +    unsigned short int *array;

>> +  struct seminfo *__buf;

>> +};

>> +#endif

>

>

> Sorry, I don't understand the comment.  Why is this definition not provided

> by the installed headers if IPC_STAT needs it?


sys/sem.h is required *not* to declare union semun; applications are
required to declare it themselves.  See
http://pubs.opengroup.org/onlinepubs/9699919799/functions/semctl.html.
Yes, this is ridiculous.  I can only guess that it was omitted by
mistake from the original incarnation of SysV semaphores, so
applications started declaring it themselves, and then POSIX was over
a barrel since redundant complete aggregate definitions aren't
allowed.

zw
Florian Weimer Jan. 2, 2017, 4:47 p.m. UTC | #3 
On 12/31/2016 05:04 PM, Zack Weinberg wrote:
> On Sat, Dec 31, 2016 at 1:07 AM, Florian Weimer <fweimer@redhat.com> wrote:

>> On 12/30/2016 09:43 PM, Tulio Magno Quites Machado Filho wrote:

>>>

>>> +/* Confirm if sys/sem.h defines semun.  */

>>> +#ifdef _SEM_SEMUN_UNDEFINED

>>> +union semun

>>> +{

>>> +  int val;

>>> +  struct semid_ds *buf;

>>> +    unsigned short int *array;

>>> +  struct seminfo *__buf;

>>> +};

>>> +#endif

>>

>>

>> Sorry, I don't understand the comment.  Why is this definition not provided

>> by the installed headers if IPC_STAT needs it?

>

> sys/sem.h is required *not* to declare union semun; applications are

> required to declare it themselves.  See

> http://pubs.opengroup.org/onlinepubs/9699919799/functions/semctl.html.

> Yes, this is ridiculous.  I can only guess that it was omitted by

> mistake from the original incarnation of SysV semaphores, so

> applications started declaring it themselves, and then POSIX was over

> a barrel since redundant complete aggregate definitions aren't

> allowed.


This still doesn't make sense.  POSIX could have standardized the union 
under a different name.  The existence struct sockaddr_storage strongly 
suggests that implementations must be able to cope with this kind of 
aliasing violation.

Florian
Tulio Magno Quites Machado Filho Jan. 2, 2017, 6:14 p.m. UTC | #4 
Florian Weimer <fweimer@redhat.com> writes:

> On 12/31/2016 05:04 PM, Zack Weinberg wrote:

>> On Sat, Dec 31, 2016 at 1:07 AM, Florian Weimer <fweimer@redhat.com> wrote:

>>> On 12/30/2016 09:43 PM, Tulio Magno Quites Machado Filho wrote:

>>>>

>>>> +/* Confirm if sys/sem.h defines semun.  */

>>>> +#ifdef _SEM_SEMUN_UNDEFINED

>>>> +union semun

>>>> +{

>>>> +  int val;

>>>> +  struct semid_ds *buf;

>>>> +    unsigned short int *array;

>>>> +  struct seminfo *__buf;

>>>> +};

>>>> +#endif

>>>

>>>

>>> Sorry, I don't understand the comment.  Why is this definition not provided

>>> by the installed headers if IPC_STAT needs it?

>>

>> sys/sem.h is required *not* to declare union semun; applications are

>> required to declare it themselves.  See

>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/semctl.html.

>> Yes, this is ridiculous.  I can only guess that it was omitted by

>> mistake from the original incarnation of SysV semaphores, so

>> applications started declaring it themselves, and then POSIX was over

>> a barrel since redundant complete aggregate definitions aren't

>> allowed.

>

> This still doesn't make sense.  POSIX could have standardized the union 

> under a different name.  The existence struct sockaddr_storage strongly 

> suggests that implementations must be able to cope with this kind of 

> aliasing violation.


Florian, could elaborate what your proposing here?

It isn't clear whether you're suggesting that this test should be changed or
if POSIX should be changed.

For the record, glibc used to define that union semun in sem.h, but had
to undefine it: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=977bfd77

-- 
Tulio Magno
Florian Weimer Jan. 2, 2017, 6:24 p.m. UTC | #5 
On 01/02/2017 07:14 PM, Tulio Magno Quites Machado Filho wrote:
> Florian Weimer <fweimer@redhat.com> writes:

>

>> On 12/31/2016 05:04 PM, Zack Weinberg wrote:

>>> On Sat, Dec 31, 2016 at 1:07 AM, Florian Weimer <fweimer@redhat.com> wrote:

>>>> On 12/30/2016 09:43 PM, Tulio Magno Quites Machado Filho wrote:

>>>>>

>>>>> +/* Confirm if sys/sem.h defines semun.  */

>>>>> +#ifdef _SEM_SEMUN_UNDEFINED

>>>>> +union semun

>>>>> +{

>>>>> +  int val;

>>>>> +  struct semid_ds *buf;

>>>>> +    unsigned short int *array;

>>>>> +  struct seminfo *__buf;

>>>>> +};

>>>>> +#endif

>>>>

>>>>

>>>> Sorry, I don't understand the comment.  Why is this definition not provided

>>>> by the installed headers if IPC_STAT needs it?

>>>

>>> sys/sem.h is required *not* to declare union semun; applications are

>>> required to declare it themselves.  See

>>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/semctl.html.

>>> Yes, this is ridiculous.  I can only guess that it was omitted by

>>> mistake from the original incarnation of SysV semaphores, so

>>> applications started declaring it themselves, and then POSIX was over

>>> a barrel since redundant complete aggregate definitions aren't

>>> allowed.

>>

>> This still doesn't make sense.  POSIX could have standardized the union

>> under a different name.  The existence struct sockaddr_storage strongly

>> suggests that implementations must be able to cope with this kind of

>> aliasing violation.

>

> Florian, could elaborate what your proposing here?

>

> It isn't clear whether you're suggesting that this test should be changed or

> if POSIX should be changed.


My comments are not relevant to the test case change.

Florian
diff mbox

Patch

diff --git a/sysvipc/Makefile b/sysvipc/Makefile
index 73bb9cf..32d64dc 100644
--- a/sysvipc/Makefile
+++ b/sysvipc/Makefile
@@ -30,7 +30,7 @@  routines := ftok \
 	    semop semget semctl semtimedop \
 	    shmat shmdt shmget shmctl
 
-tests    := test-sysvmsg
+tests    := test-sysvmsg test-sysvsem
 
 include ../Rules
 
diff --git a/sysvipc/test-sysvsem.c b/sysvipc/test-sysvsem.c
new file mode 100644
index 0000000..fd9db4f
--- /dev/null
+++ b/sysvipc/test-sysvsem.c
@@ -0,0 +1,116 @@ 
+/* Basic tests for SYSV semaphore functions.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/ipc.h>
+#include <sys/sem.h>
+
+#include <support/support.h>
+#include <support/check.h>
+#include <support/temp_file.h>
+
+/* These are for the temporary file we generate.  */
+static char *name;
+static int semid;
+
+static void
+remove_sem (void)
+{
+  /* Enforce message queue removal in case of early test failure.
+     Ignore error since the sem may already have being removed.  */
+  semctl (semid, 0, IPC_RMID, 0);
+}
+
+static void
+do_prepare (int argc, char *argv[])
+{
+  int fd = create_temp_file ("tst-sysvsem.", &name);
+  if (fd == -1)
+    FAIL_EXIT1 ("cannot create temporary file (errno=%d)", errno);
+}
+
+#define PREPARE do_prepare
+
+/* It is not an extensive test, but rather a functional one aimed to check
+   correct parameter passing on kernel.  */
+
+#define SEM_MODE 0644
+
+static int
+do_test (void)
+{
+  atexit (remove_sem);
+
+  key_t key = ftok (name, 'G');
+  if (key == -1)
+    FAIL_EXIT1 ("ftok failed");
+
+  semid = semget(key, 1, IPC_CREAT | IPC_EXCL | SEM_MODE);
+  if (semid == -1)
+    {
+      if (errno == ENOSYS)
+	FAIL_UNSUPPORTED ("msgget not supported");
+      FAIL_EXIT1 ("semget failed (errno=%d)", errno);
+    }
+
+  /* Get semaphore kernel information and do some sanity checks.  */
+  struct semid_ds seminfo;
+  if (semctl (semid, 0, IPC_STAT, &seminfo) == -1)
+    FAIL_EXIT1 ("semctl with IPC_STAT failed (errno=%d)", errno);
+
+  if (seminfo.sem_perm.__key != key)
+    FAIL_EXIT1 ("semid_ds::sem_perm::key (%d) != %d",
+		(int) seminfo.sem_perm.__key, (int) key);
+  if (seminfo.sem_perm.mode != SEM_MODE)
+    FAIL_EXIT1 ("semid_ds::sem_perm::mode (%o) != %o",
+		seminfo.sem_perm.mode, SEM_MODE);
+  if (seminfo.sem_nsems != 1)
+    FAIL_EXIT1 ("semid_ds::sem_nsems (%lu) != 1",
+		(long unsigned) seminfo.sem_nsems);
+
+  /* Some lock/unlock basic tests.  */
+  struct sembuf sb1 = { 0, 1, 0 };
+  if (semop (semid, &sb1, 1) == -1)
+    FAIL_EXIT1 ("semop failed (errno=%i)", errno);
+
+  struct sembuf sb2 = { 0, -1, 0 };
+  if (semop (semid, &sb2, 1) == -1)
+    FAIL_EXIT1 ("semop failed (errno=%i)", errno);
+
+#ifdef _GNU_SOURCE
+  /* Set a time for half a second.  The semaphore operation should timeout
+     with EAGAIN.  */
+  struct timespec ts = { 0 /* sec */, 500000000 /* nsec */ };
+  if (semtimedop (semid, &sb2, 1, &ts) != -1
+      || (errno != EAGAIN && errno != ENOSYS))
+    FAIL_EXIT1 ("semtimedop succeed or returned errno != {EAGAIN,ENOSYS} "
+		"(errno=%i)", errno);
+#endif
+
+  /* Finally free up the semnaphore resource.  */
+  if (semctl (semid, 0, IPC_RMID, 0) == -1)
+    FAIL_EXIT1 ("semctl failed (errno=%d)", errno);
+
+  return 0;
+}
+
+#include <support/test-driver.c>