All of lore.kernel.org
 help / color / mirror / Atom feed
* [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting
       [not found] <mailman.4536.1268063545.8399.xenomai-git@xenomai.org>
@ 2010-03-08 17:28 ` Jan Kiszka
  2010-03-08 17:31   ` Jan Kiszka
  2010-03-08 17:35   ` Gilles Chanteperdrix
  0 siblings, 2 replies; 5+ messages in thread
From: Jan Kiszka @ 2010-03-08 17:28 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai-core

xenomai-git-request@domain.hid wrote:
> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
> index 6c3ec3a..2a97a2d 100644
> --- a/ksrc/skins/posix/syscall.c
> +++ b/ksrc/skins/posix/syscall.c
> @@ -1520,7 +1520,7 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>  	union __xeno_mutex mx, *umx;
>  	unsigned timed, count;
>  	struct timespec ts;
> -	int err;
> +	int err, perr = 0;
>  
>  	ucnd = (union __xeno_cond *)__xn_reg_arg1(regs);
>  	umx = (union __xeno_mutex *)__xn_reg_arg2(regs);
> @@ -1560,7 +1560,10 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>  						    &mx.shadow_mutex,
>  						    &count, timed, XN_INFINITE);
>  
> -	if (err == 0 || err == ETIMEDOUT) {
> +	switch(err) {
> +	case 0:
> +	case ETIMEDOUT:
> +		perr = errno = err;
>  		err = -pse51_cond_timedwait_epilogue(cur, &cnd.shadow_cond,
>  					    	    &mx.shadow_mutex, count);
>  		if (err == 0 &&
> @@ -1569,14 +1572,20 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>  					   &mx.shadow_mutex.lockcnt,
>  					   sizeof(umx->shadow_mutex.lockcnt)))
>  			return -EFAULT;
> +		break;
> +
> +	case EINTR:
> +		perr = err;

Minor cleanup: This is not needed as err != 0, so perr will not be
evaluated anymore. Same for native.

> +		errno = 0;	/* epilogue should return 0. */
> +		break;
>  	}
>  
> -	if (err == EINTR
> -	    && __xn_safe_copy_to_user((void __user *)__xn_reg_arg3(regs),
> -				      &count, sizeof(count)))
> +	if (err == EINTR 
> +	    &&__xn_safe_copy_to_user((void __user *)__xn_reg_arg3(regs),
> +				     &count, sizeof(count)))
>  			return -EFAULT;
> -	
> -	return -err;
> +
> +	return err == 0 ? -perr : -err;
>  }
>  
>  /* pthread_cond_wait_epilogue(cond, mutex, count) */

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting
  2010-03-08 17:28 ` [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting Jan Kiszka
@ 2010-03-08 17:31   ` Jan Kiszka
  2010-03-08 17:35   ` Gilles Chanteperdrix
  1 sibling, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2010-03-08 17:31 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai-core

Jan Kiszka wrote:
> xenomai-git-request@domain.hid wrote:
>> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
>> index 6c3ec3a..2a97a2d 100644
>> --- a/ksrc/skins/posix/syscall.c
>> +++ b/ksrc/skins/posix/syscall.c
>> @@ -1520,7 +1520,7 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>  	union __xeno_mutex mx, *umx;
>>  	unsigned timed, count;
>>  	struct timespec ts;
>> -	int err;
>> +	int err, perr = 0;
>>  
>>  	ucnd = (union __xeno_cond *)__xn_reg_arg1(regs);
>>  	umx = (union __xeno_mutex *)__xn_reg_arg2(regs);
>> @@ -1560,7 +1560,10 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>  						    &mx.shadow_mutex,
>>  						    &count, timed, XN_INFINITE);
>>  
>> -	if (err == 0 || err == ETIMEDOUT) {
>> +	switch(err) {
>> +	case 0:
>> +	case ETIMEDOUT:
>> +		perr = errno = err;
>>  		err = -pse51_cond_timedwait_epilogue(cur, &cnd.shadow_cond,
>>  					    	    &mx.shadow_mutex, count);
>>  		if (err == 0 &&
>> @@ -1569,14 +1572,20 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>  					   &mx.shadow_mutex.lockcnt,
>>  					   sizeof(umx->shadow_mutex.lockcnt)))
>>  			return -EFAULT;
>> +		break;
>> +
>> +	case EINTR:
>> +		perr = err;
> 
> Minor cleanup: This is not needed as err != 0, so perr will not be
> evaluated anymore. Same for native.

In fact, you could even limit perr = err to "case 0:".

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting
  2010-03-08 17:28 ` [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting Jan Kiszka
  2010-03-08 17:31   ` Jan Kiszka
@ 2010-03-08 17:35   ` Gilles Chanteperdrix
  2010-03-08 17:39     ` Jan Kiszka
  1 sibling, 1 reply; 5+ messages in thread
From: Gilles Chanteperdrix @ 2010-03-08 17:35 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai-core

Jan Kiszka wrote:
> xenomai-git-request@domain.hid wrote:
>> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
>> index 6c3ec3a..2a97a2d 100644
>> --- a/ksrc/skins/posix/syscall.c
>> +++ b/ksrc/skins/posix/syscall.c
>> @@ -1520,7 +1520,7 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>  	union __xeno_mutex mx, *umx;
>>  	unsigned timed, count;
>>  	struct timespec ts;
>> -	int err;
>> +	int err, perr = 0;
>>  
>>  	ucnd = (union __xeno_cond *)__xn_reg_arg1(regs);
>>  	umx = (union __xeno_mutex *)__xn_reg_arg2(regs);
>> @@ -1560,7 +1560,10 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>  						    &mx.shadow_mutex,
>>  						    &count, timed, XN_INFINITE);
>>  
>> -	if (err == 0 || err == ETIMEDOUT) {
>> +	switch(err) {
>> +	case 0:
>> +	case ETIMEDOUT:
>> +		perr = errno = err;
>>  		err = -pse51_cond_timedwait_epilogue(cur, &cnd.shadow_cond,
>>  					    	    &mx.shadow_mutex, count);
>>  		if (err == 0 &&
>> @@ -1569,14 +1572,20 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>  					   &mx.shadow_mutex.lockcnt,
>>  					   sizeof(umx->shadow_mutex.lockcnt)))
>>  			return -EFAULT;
>> +		break;
>> +
>> +	case EINTR:
>> +		perr = err;
> 
> Minor cleanup: This is not needed as err != 0, so perr will not be
> evaluated anymore. Same for native.

No, err may be 0, if the epilogue returns 0.

The following table should explains how I came to this solution:

perr		err		wanted		err == 0 ? perr : err
0		0		0          	0
0		EINTR		EINTR      	EINTR
0		*		*          	*
ETIMEDOUT  	0		ETIMEDOUT  	ETIMEDOUNT
ETIMEDOUT  	EINTR		EINTR      	EINTR
		    	  (epi ETIMEDOUT)
ETIMEDOUT  	*		*		*
EINTR	   	-(perr)		EINTR      	EINTR
*	   	-(perr)		*		*


-- 
					    Gilles.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting
  2010-03-08 17:35   ` Gilles Chanteperdrix
@ 2010-03-08 17:39     ` Jan Kiszka
  2010-03-08 17:47       ` Gilles Chanteperdrix
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2010-03-08 17:39 UTC (permalink / raw)
  To: Gilles Chanteperdrix; +Cc: xenomai-core

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> xenomai-git-request@domain.hid wrote:
>>> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
>>> index 6c3ec3a..2a97a2d 100644
>>> --- a/ksrc/skins/posix/syscall.c
>>> +++ b/ksrc/skins/posix/syscall.c
>>> @@ -1520,7 +1520,7 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>>  	union __xeno_mutex mx, *umx;
>>>  	unsigned timed, count;
>>>  	struct timespec ts;
>>> -	int err;
>>> +	int err, perr = 0;
>>>  
>>>  	ucnd = (union __xeno_cond *)__xn_reg_arg1(regs);
>>>  	umx = (union __xeno_mutex *)__xn_reg_arg2(regs);
>>> @@ -1560,7 +1560,10 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>>  						    &mx.shadow_mutex,
>>>  						    &count, timed, XN_INFINITE);
>>>  
>>> -	if (err == 0 || err == ETIMEDOUT) {
>>> +	switch(err) {
>>> +	case 0:
>>> +	case ETIMEDOUT:
>>> +		perr = errno = err;
>>>  		err = -pse51_cond_timedwait_epilogue(cur, &cnd.shadow_cond,
>>>  					    	    &mx.shadow_mutex, count);
>>>  		if (err == 0 &&
>>> @@ -1569,14 +1572,20 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>>  					   &mx.shadow_mutex.lockcnt,
>>>  					   sizeof(umx->shadow_mutex.lockcnt)))
>>>  			return -EFAULT;
>>> +		break;
>>> +
>>> +	case EINTR:
>>> +		perr = err;
>> Minor cleanup: This is not needed as err != 0, so perr will not be
>> evaluated anymore. Same for native.
> 
> No, err may be 0, if the epilogue returns 0.
> 
> The following table should explains how I came to this solution:
> 
> perr		err		wanted		err == 0 ? perr : err
> 0		0		0          	0
> 0		EINTR		EINTR      	EINTR
> 0		*		*          	*
> ETIMEDOUT  	0		ETIMEDOUT  	ETIMEDOUNT
> ETIMEDOUT  	EINTR		EINTR      	EINTR
> 		    	  (epi ETIMEDOUT)
> ETIMEDOUT  	*		*		*
> EINTR	   	-(perr)		EINTR      	EINTR
> *	   	-(perr)		*		*
> 

Right for my second suggestion. But err = [-]EINTR from the prologue
will remain != 0.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting
  2010-03-08 17:39     ` Jan Kiszka
@ 2010-03-08 17:47       ` Gilles Chanteperdrix
  0 siblings, 0 replies; 5+ messages in thread
From: Gilles Chanteperdrix @ 2010-03-08 17:47 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai-core

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> xenomai-git-request@domain.hid wrote:
>>>> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
>>>> index 6c3ec3a..2a97a2d 100644
>>>> --- a/ksrc/skins/posix/syscall.c
>>>> +++ b/ksrc/skins/posix/syscall.c
>>>> @@ -1520,7 +1520,7 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>>>  	union __xeno_mutex mx, *umx;
>>>>  	unsigned timed, count;
>>>>  	struct timespec ts;
>>>> -	int err;
>>>> +	int err, perr = 0;
>>>>  
>>>>  	ucnd = (union __xeno_cond *)__xn_reg_arg1(regs);
>>>>  	umx = (union __xeno_mutex *)__xn_reg_arg2(regs);
>>>> @@ -1560,7 +1560,10 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>>>  						    &mx.shadow_mutex,
>>>>  						    &count, timed, XN_INFINITE);
>>>>  
>>>> -	if (err == 0 || err == ETIMEDOUT) {
>>>> +	switch(err) {
>>>> +	case 0:
>>>> +	case ETIMEDOUT:
>>>> +		perr = errno = err;
>>>>  		err = -pse51_cond_timedwait_epilogue(cur, &cnd.shadow_cond,
>>>>  					    	    &mx.shadow_mutex, count);
>>>>  		if (err == 0 &&
>>>> @@ -1569,14 +1572,20 @@ static int __pthread_cond_wait_prologue(struct pt_regs *regs)
>>>>  					   &mx.shadow_mutex.lockcnt,
>>>>  					   sizeof(umx->shadow_mutex.lockcnt)))
>>>>  			return -EFAULT;
>>>> +		break;
>>>> +
>>>> +	case EINTR:
>>>> +		perr = err;
>>> Minor cleanup: This is not needed as err != 0, so perr will not be
>>> evaluated anymore. Same for native.
>> No, err may be 0, if the epilogue returns 0.
>>
>> The following table should explains how I came to this solution:
>>
>> perr		err		wanted		err == 0 ? perr : err
>> 0		0		0          	0
>> 0		EINTR		EINTR      	EINTR
>> 0		*		*          	*
>> ETIMEDOUT  	0		ETIMEDOUT  	ETIMEDOUNT
>> ETIMEDOUT  	EINTR		EINTR      	EINTR
>> 		    	  (epi ETIMEDOUT)
>> ETIMEDOUT  	*		*		*
>> EINTR	   	-(perr)		EINTR      	EINTR
>> *	   	-(perr)		*		*
>>
> 
> Right for my second suggestion. But err = [-]EINTR from the prologue
> will remain != 0.

Yes, Ok. Maybe a (failed) attempt to get gcc to stop whining about perr
being used non-initialized, which remained in the way. Which I finally
solved by initializing the local variable with 0.

-- 
					    Gilles.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-03-08 17:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <mailman.4536.1268063545.8399.xenomai-git@xenomai.org>
2010-03-08 17:28 ` [Xenomai-core] [Xenomai-git] posix: fix pthread_cond_*wait return value overwriting Jan Kiszka
2010-03-08 17:31   ` Jan Kiszka
2010-03-08 17:35   ` Gilles Chanteperdrix
2010-03-08 17:39     ` Jan Kiszka
2010-03-08 17:47       ` Gilles Chanteperdrix

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.