Message ID | 1432312942.16668.92.camel@ubuntu-sellcey |
---|---|
State | Superseded |
Headers |
Received: (qmail 51766 invoked by alias); 22 May 2015 16:42:34 -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 51754 invoked by uid 89); 22 May 2015 16:42:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mailapp01.imgtec.com Message-ID: <1432312942.16668.92.camel@ubuntu-sellcey> Subject: [PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal From: Steve Ellcey <sellcey@imgtec.com> Reply-To: <sellcey@imgtec.com> To: GNU C Library <libc-alpha@sourceware.org> CC: Roland McGrath <roland@hack.frob.com> Date: Fri, 22 May 2015 09:42:22 -0700 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 |
Commit Message
Steve Ellcey
May 22, 2015, 4:42 p.m. UTC
Roland's patch (https://sourceware.org/ml/libc-alpha/2015-05/msg00464.html) to set tid field to a unique value removed the declaration of err [INTERNAL_SYSCALL_DECL (err);] from __pthread_initialize_minimal_internal, but there are other uses of err in other INTERNAL_SYSCALL's in __pthread_initialize_minimal_internal so this broke the build glibc build for MIPS (and presumably other platforms). Here is a patch to put the declaration back. I think the only question is exactly where this declaration should go. I initially put it right in front of the first INTERNAL_SYSCALL call but I noticed that that is inside of an ifdef and there are other INTERNAL_SYSCALL uses (with err) that are in different ifdef's so I moved the declaration outside of any ifdef's. This fixed my build problem. OK to checkin? Steve Ellcey sellcey@imgtec.com 2015-05-22 Steve Ellcey <sellcey@imgtec.com> * nptl/nptl-init.c (__pthread_initialize_minimal_internal): Add declaration of err that was removed in earlier patch.
Comments
On 05/22/2015 12:42 PM, Steve Ellcey wrote: > Roland's patch (https://sourceware.org/ml/libc-alpha/2015-05/msg00464.html) > to set tid field to a unique value removed the declaration of err > [INTERNAL_SYSCALL_DECL (err);] from __pthread_initialize_minimal_internal, > but there are other uses of err in other INTERNAL_SYSCALL's in > __pthread_initialize_minimal_internal so this broke the build glibc build > for MIPS (and presumably other platforms). > > Here is a patch to put the declaration back. I think the only question > is exactly where this declaration should go. I initially put it right > in front of the first INTERNAL_SYSCALL call but I noticed that that is inside > of an ifdef and there are other INTERNAL_SYSCALL uses (with err) that > are in different ifdef's so I moved the declaration outside of any ifdef's. > This fixed my build problem. OK to checkin? > > Steve Ellcey > sellcey@imgtec.com > > > > 2015-05-22 Steve Ellcey <sellcey@imgtec.com> > > * nptl/nptl-init.c (__pthread_initialize_minimal_internal): > Add declaration of err that was removed in earlier patch. > > > > diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c > index 5b8d931..3bfb478 100644 > --- a/nptl/nptl-init.c > +++ b/nptl/nptl-init.c > @@ -326,6 +326,7 @@ __pthread_initialize_minimal_internal (void) > pd->robust_prev = &pd->robust_head; > #endif > pd->robust_head.list = &pd->robust_head; > + INTERNAL_SYSCALL_DECL (err); Why isn't this inside the inner ifdef? > #ifdef __NR_set_robust_list > pd->robust_head.futex_offset = (offsetof (pthread_mutex_t, __data.__lock) > - offsetof (pthread_mutex_t, As Florian mentioned to Raj, the definition of the err decl should be in the same scope as the syscall that uses it. Did I miss something? Cheers, Carlos.
On Sat, May 23, 2015 at 8:04 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 05/22/2015 12:42 PM, Steve Ellcey wrote: >> Roland's patch (https://sourceware.org/ml/libc-alpha/2015-05/msg00464.html) >> to set tid field to a unique value removed the declaration of err >> [INTERNAL_SYSCALL_DECL (err);] from __pthread_initialize_minimal_internal, >> but there are other uses of err in other INTERNAL_SYSCALL's in >> __pthread_initialize_minimal_internal so this broke the build glibc build >> for MIPS (and presumably other platforms). >> >> Here is a patch to put the declaration back. I think the only question >> is exactly where this declaration should go. I initially put it right >> in front of the first INTERNAL_SYSCALL call but I noticed that that is inside >> of an ifdef and there are other INTERNAL_SYSCALL uses (with err) that >> are in different ifdef's so I moved the declaration outside of any ifdef's. >> This fixed my build problem. OK to checkin? >> >> Steve Ellcey >> sellcey@imgtec.com >> >> >> >> 2015-05-22 Steve Ellcey <sellcey@imgtec.com> >> >> * nptl/nptl-init.c (__pthread_initialize_minimal_internal): >> Add declaration of err that was removed in earlier patch. >> >> >> >> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c >> index 5b8d931..3bfb478 100644 >> --- a/nptl/nptl-init.c >> +++ b/nptl/nptl-init.c >> @@ -326,6 +326,7 @@ __pthread_initialize_minimal_internal (void) >> pd->robust_prev = &pd->robust_head; >> #endif >> pd->robust_head.list = &pd->robust_head; >> + INTERNAL_SYSCALL_DECL (err); > > Why isn't this inside the inner ifdef? > >> #ifdef __NR_set_robust_list >> pd->robust_head.futex_offset = (offsetof (pthread_mutex_t, __data.__lock) >> - offsetof (pthread_mutex_t, > > As Florian mentioned to Raj, the definition of the err decl should > be in the same scope as the syscall that uses it. > > Did I miss something? I think Roland posted another patch which should already fix this issue. > > Cheers, > Carlos. >
On Sat, 2015-05-23 at 23:04 -0400, Carlos O'Donell wrote: > Why isn't this inside the inner ifdef? > > > #ifdef __NR_set_robust_list > > pd->robust_head.futex_offset = (offsetof (pthread_mutex_t, __data.__lock) > > - offsetof (pthread_mutex_t, > > As Florian mentioned to Raj, the definition of the err decl should > be in the same scope as the syscall that uses it. > > Did I miss something? > > Cheers, > Carlos. This patch is no longer needed due to Roland's patch but the reason I didn't put INTERNAL_SYSCALL_DECL in the same scope as INTERNAL_SYSCALL was because there was multiple INTERNAL_SYSCALL calls in __pthread_initialize_minimal_internal and I wanted one INTERNAL_SYSCALL_DECL to cover them all. That was basically what we had before. I guess the right way (and what Roland checked in) is to have one INTERNAL_SYSCALL_DECL for each scope with an INTERNAL_SYSCALL and if you have two INTERNAL_SYSCALL's in different ifdefs but in the same C scope then use brackets to make different scopes so you can have a INTERNAL_SYSCALL_DECL with each INTERNAL_SYSCALL. I.e. do not do this: INTERNAL_SYSCALL_DECL #if A INTERNAL_SYSCALL() #endif #if B INTERNAL_SYSCALL() #endif but instead do this: #if A { INTERNAL_SYSCALL_DECL INTERNAL_SYSCALL() } #endif #if B { INTERNAL_SYSCALL_DECL INTERNAL_SYSCALL() } #endif Steve Ellcey sellcey@imgtec.com
On 05/26/2015 12:42 PM, Steve Ellcey wrote: > On Sat, 2015-05-23 at 23:04 -0400, Carlos O'Donell wrote: > >> Why isn't this inside the inner ifdef? >> >>> #ifdef __NR_set_robust_list >>> pd->robust_head.futex_offset = (offsetof (pthread_mutex_t, __data.__lock) >>> - offsetof (pthread_mutex_t, >> >> As Florian mentioned to Raj, the definition of the err decl should >> be in the same scope as the syscall that uses it. >> >> Did I miss something? >> >> Cheers, >> Carlos. > > This patch is no longer needed due to Roland's patch but the reason I > didn't put INTERNAL_SYSCALL_DECL in the same scope as INTERNAL_SYSCALL > was because there was multiple INTERNAL_SYSCALL calls in > __pthread_initialize_minimal_internal and I wanted one > INTERNAL_SYSCALL_DECL to cover them all. That was basically what we had > before. I guess the right way (and what Roland checked in) is to have > one INTERNAL_SYSCALL_DECL for each scope with an INTERNAL_SYSCALL and if > you have two INTERNAL_SYSCALL's in different ifdefs but in the same C > scope then use brackets to make different scopes so you can have a > INTERNAL_SYSCALL_DECL with each INTERNAL_SYSCALL. > > I.e. do not do this: > > INTERNAL_SYSCALL_DECL > #if A > INTERNAL_SYSCALL() > #endif > #if B > INTERNAL_SYSCALL() > #endif > > > but instead do this: > > > #if A > { > INTERNAL_SYSCALL_DECL > INTERNAL_SYSCALL() > } > #endif > #if B > { > INTERNAL_SYSCALL_DECL > INTERNAL_SYSCALL() > } > #endif Agreed. I did see later that Roland checked in a solution. Cheers, Carlos.
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index 5b8d931..3bfb478 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -326,6 +326,7 @@ __pthread_initialize_minimal_internal (void) pd->robust_prev = &pd->robust_head; #endif pd->robust_head.list = &pd->robust_head; + INTERNAL_SYSCALL_DECL (err); #ifdef __NR_set_robust_list pd->robust_head.futex_offset = (offsetof (pthread_mutex_t, __data.__lock) - offsetof (pthread_mutex_t,