[PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal

[PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal

[Steve Ellcey]

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);
 #ifdef __NR_set_robust_list
   pd->robust_head.futex_offset = (offsetof (pthread_mutex_t, __data.__lock)
 				  - offsetof (pthread_mutex_t,

Re: [PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal

[Carlos O'Donell]

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.

Re: [PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal

[Khem Raj]

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.
>

Re: [PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal

[Steve Ellcey]

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
  

Re: [PATCH] Fix missing err declaration in __pthread_initialize_minimal_internal

[Carlos O'Donell]

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.