Message ID | 1483130610-2500-1-git-send-email-tuliom@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
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
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
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
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
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 --git a/sysvipc/test-sysvsem.c b/sysvipc/test-sysvsem.c index fd9db4f..6cc0c46 100644 --- a/sysvipc/test-sysvsem.c +++ b/sysvipc/test-sysvsem.c @@ -28,6 +28,17 @@ #include <support/check.h> #include <support/temp_file.h> +/* 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 + /* These are for the temporary file we generate. */ static char *name; static int semid; @@ -74,7 +85,10 @@ do_test (void) /* Get semaphore kernel information and do some sanity checks. */ struct semid_ds seminfo; - if (semctl (semid, 0, IPC_STAT, &seminfo) == -1) + + union semun semarg; + semarg.buf = &seminfo; + if (semctl (semid, 0, IPC_STAT, semarg) == -1) FAIL_EXIT1 ("semctl with IPC_STAT failed (errno=%d)", errno); if (seminfo.sem_perm.__key != key)