| Message ID | 1483130610-2500-1-git-send-email-tuliom@linux.vnet.ibm.com |
|---|---|
| State | Committed |
| Delegated to: | Adhemerval Zanella Netto |
| Headers |
Received: (qmail 52062 invoked by alias); 30 Dec 2016 20:44:02 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 52038 invoked by uid 89); 30 Dec 2016 20:44:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=Confirm, sk:tuliom@, U*tuliom, sk:tuliom X-HELO: mx0a-001b2d01.pphosted.com From: "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> To: libc-alpha@sourceware.org, adhemerval.zanella@linaro.org Subject: [PATCH] Fix argument passing in sysvipc/test-sysvsem Date: Fri, 30 Dec 2016 18:43:30 -0200 In-Reply-To: <1482847286-29933-13-git-send-email-adhemerval.zanella@linaro.org> References: <1482847286-29933-13-git-send-email-adhemerval.zanella@linaro.org> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16123020-1523-0000-0000-0000025EC35B X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16123020-1524-0000-0000-000029EADC7A Message-Id: <1483130610-2500-1-git-send-email-tuliom@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-12-30_14:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1612300323 |
Commit Message
Tulio Magno Quites Machado Filho
Dec. 30, 2016, 8:43 p.m. UTC
The command IPC_STAT of semctl() expects an union semun in its fourth argument instead of struct semid_ds *. This can cause failures on ppc. Tested on ppc. 2016-12-30 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> * sysvipc/test-sysvsem.c: Define union semun. (do_test): Pass union semun to semctl() instead of struct semid_ds *. --- sysvipc/test-sysvsem.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
Comments
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)