Message ID | 1432312942.16668.92.camel@ubuntu-sellcey |
---|---|
State | New |
Headers | show |
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,