Re: [PATCH] isdn: Return -EINTR in gigaset_start() if locking attempts fails.

2012-03-17 Thread David Miller
From: santosh nayak santoshprasadna...@gmail.com
Date: Fri, 16 Mar 2012 18:40:13 +0530

 We have 3 callers: gigaset_probe(), gigaset_tty_open() and
 gigaset_probe(). Each caller tries to free allocated memory
 if lock fails. This is possible if we returns -EINTR.

Look again at the callers.

They interpret 0 as an error, so your patch would break the driver.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] isdn: Return -EINTR in gigaset_start() if locking attempts fails.

2012-03-17 Thread Tilman Schmidt
Am 16.03.2012 14:10, schrieb santosh nayak:
 From: Santosh Nayak santoshprasadna...@gmail.com
 
 If locking attempt was interrupted by a signal then we should
 return -EINTR so that caller can take appropriate action.

NACK.

The return value of gigaset_start(), as documented in its
header comment, is:
 *  1 - success, 0 - error
Its callers rely on this. If you want to change it, you must
also change the documentation and the callers accordingly.
In its current form the patch would just break the driver.

Thanks,
Tilman

-- 
Tilman SchmidtE-Mail: til...@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] isdn: Return -EINTR in gigaset_start() if locking attempts fails.

2012-03-17 Thread santosh prasad nayak
Yes. You are right.

Caller is interpreting 0 in opposite way of normal sequence.
Thats why I misunderstood it.

In general, 0 means success and on error we return -ve.
Here its opposite.




regards
Santosh



On Sat, Mar 17, 2012 at 11:48 AM, David Miller da...@davemloft.net wrote:
 From: santosh nayak santoshprasadna...@gmail.com
 Date: Fri, 16 Mar 2012 18:40:13 +0530

 We have 3 callers: gigaset_probe(), gigaset_tty_open() and
 gigaset_probe(). Each caller tries to free allocated memory
 if lock fails. This is possible if we returns -EINTR.

 Look again at the callers.

 They interpret 0 as an error, so your patch would break the driver.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] isdn: Return -EINTR in gigaset_start() if locking attempts fails.

2012-03-17 Thread David Miller
From: santosh prasad nayak santoshprasadna...@gmail.com
Date: Sat, 17 Mar 2012 21:26:14 +0530

 Caller is interpreting 0 in opposite way of normal sequence.
 Thats why I misunderstood it.

The simple fact is that you didn't even look at the code at the call
sites when you wrote this patch, and that's the first thing anyone is
going to do when reviewing it.

Therefore your laziness results in more useless work for other people.
I just wanted to point out how selfish and anti-social this kind of
behavior is.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] isdn: Return -EINTR in gigaset_start() if locking attempts fails.

2012-03-17 Thread Julia Lawall

On Sat, 17 Mar 2012, David Miller wrote:


From: santosh prasad nayak santoshprasadna...@gmail.com
Date: Sat, 17 Mar 2012 21:26:14 +0530


Caller is interpreting 0 in opposite way of normal sequence.
Thats why I misunderstood it.


The simple fact is that you didn't even look at the code at the call
sites when you wrote this patch, and that's the first thing anyone is
going to do when reviewing it.

Therefore your laziness results in more useless work for other people.
I just wanted to point out how selfish and anti-social this kind of
behavior is.


Not to pour too much salt on a wound, but just 5 lines above the patch 
site is the comment:


 * Return value:
 *  1 - success, 0 - error

And the error label also returns 0.  So there is a lot of local 
information that one can use to see what protocol the function follows.


That said, it's a bit too bad that two protocols have to coexist.

julia
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] isdn: Return -EINTR in gigaset_start() if locking attempts fails.

2012-03-16 Thread santosh nayak
From: Santosh Nayak santoshprasadna...@gmail.com

If locking attempt was interrupted by a signal then we should
return -EINTR so that caller can take appropriate action.

We have 3 callers: gigaset_probe(), gigaset_tty_open() and
gigaset_probe(). Each caller tries to free allocated memory
if lock fails. This is possible if we returns -EINTR.

Signed-off-by: Santosh Nayak santoshprasadna...@gmail.com
---
 drivers/isdn/gigaset/common.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 7679270..2d10f3a 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -903,7 +903,7 @@ int gigaset_start(struct cardstate *cs)
unsigned long flags;
 
if (mutex_lock_interruptible(cs-mutex))
-   return 0;
+   return -EINTR;
 
spin_lock_irqsave(cs-lock, flags);
cs-connected = 1;
-- 
1.7.4.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html