[PATCH 1/3] Return ENODEV instead of EINVAL when trying to open ACM device.

[PATCH 1/3] Return ENODEV instead of EINVAL when trying to open ACM device.

[Thadeu Lima de Souza Cascardo]

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/usb/class/cdc-acm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 38bfdb0..02eb60b 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -550,7 +550,7 @@ static void acm_waker(struct work_struct *waker)
 static int acm_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct acm *acm;
-	int rv = -EINVAL;
+	int rv = -ENODEV;
 	int i;
 	dbg("Entering acm_tty_open.");
 
-- 
1.6.3

[PATCH 2/3] Fix oops when closing ACM tty device right after open has failed.

[Thadeu Lima de Souza Cascardo]

This commit 10077d4a6674f535abdbe25cdecb1202af7948f1 has stopped
checking if there was a valid acm device associated to the tty, which is
not true right after open fails and tty subsystem tries to close the
device.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/usb/class/cdc-acm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 02eb60b..3f10459 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -677,7 +677,7 @@ static void acm_tty_close(struct tty_struct *tty, struct file *filp)
 
 	/* Perform the closing process and see if we need to do the hardware
 	   shutdown */
-	if (tty_port_close_start(&acm->port, tty, filp) == 0)
+	if (!acm || tty_port_close_start(&acm->port, tty, filp) == 0)
 		return;
 	acm_port_down(acm, 0);
 	tty_port_close_end(&acm->port, tty);
-- 
1.6.3

[PATCH 3/3] Fix oops when unexisting usb serial device is opened.

[Thadeu Lima de Souza Cascardo]

This commit 335f8514f200e63d689113d29cb7253a5c282967 has stopped
properly checking if there is any usb serial associated with the tty in
the close function. It happens the close function is called by releasing
the terminal right after opening the device fails.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/usb/serial/usb-serial.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index d595aa5..a842164 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -333,6 +333,9 @@ static void serial_close(struct tty_struct *tty, struct file *filp)
 {
 	struct usb_serial_port *port = tty->driver_data;
 
+	if (!port)
+		return;
+
 	dbg("%s - port %d", __func__, port->number);
 
 
-- 
1.6.3

Re: [PATCH 3/3] Fix oops when unexisting usb serial device is opened.

[Greg KH]

On Wed, Jun 24, 2009 at 08:31:17AM -0300, Thadeu Lima de Souza Cascardo wrote:
> This commit 335f8514f200e63d689113d29cb7253a5c282967 has stopped
> properly checking if there is any usb serial associated with the tty in
> the close function. It happens the close function is called by releasing
> the terminal right after opening the device fails.

Why would open fail?

thanks,

greg k-h

Re: [PATCH 2/3] Fix oops when closing ACM tty device right after open has failed.

[Greg KH]

On Wed, Jun 24, 2009 at 08:31:16AM -0300, Thadeu Lima de Souza Cascardo wrote:
> This commit 10077d4a6674f535abdbe25cdecb1202af7948f1 has stopped

Please don't reference git commit ids that are not in Linus's tree, as
it doesn't make any sense to anyone.

> checking if there was a valid acm device associated to the tty, which is
> not true right after open fails and tty subsystem tries to close the
> device.

Why would open fail?

thanks,

greg k-h

Re: [PATCH 2/3] Fix oops when closing ACM tty device right after open has failed.

[Thadeu Lima de Souza Cascardo]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Wed, Jun 24, 2009 at 01:41:06PM -0700, Greg KH wrote:
> On Wed, Jun 24, 2009 at 08:31:16AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > This commit 10077d4a6674f535abdbe25cdecb1202af7948f1 has stopped
> 
> Please don't reference git commit ids that are not in Linus's tree, as
> it doesn't make any sense to anyone.
> 

It is in Linus's tree. git log torvalds/master..10077d4 gives me
nothing.

cascardo@vespa:~/linux$ git remote show torvalds
* remote torvalds
  URL:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/

> > checking if there was a valid acm device associated to the tty, which is
> > not true right after open fails and tty subsystem tries to close the
> > device.
> 
> Why would open fail?
> 

Explained in a second version submitted already, but basically because
there's no such device.

> thanks,
> 
> greg k-h

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] Fix oops when closing ACM tty device right after open has failed.

[Greg KH]

On Wed, Jun 24, 2009 at 06:35:23PM -0300, Thadeu Lima de Souza Cascardo wrote:
> On Wed, Jun 24, 2009 at 01:41:06PM -0700, Greg KH wrote:
> > On Wed, Jun 24, 2009 at 08:31:16AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > > This commit 10077d4a6674f535abdbe25cdecb1202af7948f1 has stopped
> > 
> > Please don't reference git commit ids that are not in Linus's tree, as
> > it doesn't make any sense to anyone.
> > 
> 
> It is in Linus's tree. git log torvalds/master..10077d4 gives me
> nothing.

Oh, ok, your phrase was a bit confusing.

It should say:
	The commit XXXXX has stopped...

thanks,

greg k-h

Re: [PATCH 1/3] Return ENODEV instead of EINVAL when trying to open ACM device.

[Greg KH]

On Wed, Jun 24, 2009 at 08:31:15AM -0300, Thadeu Lima de Souza Cascardo wrote:
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> ---
>  drivers/usb/class/cdc-acm.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 38bfdb0..02eb60b 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -550,7 +550,7 @@ static void acm_waker(struct work_struct *waker)
>  static int acm_tty_open(struct tty_struct *tty, struct file *filp)
>  {
>  	struct acm *acm;
> -	int rv = -EINVAL;
> +	int rv = -ENODEV;

Why is this needed?

thanks,

greg k-h

Re: [PATCH 1/3] Return ENODEV instead of EINVAL when trying to open ACM device.

[Oliver Neukum]

Am Mittwoch, 24. Juni 2009 22:40:15 schrieb Greg KH:
> On Wed, Jun 24, 2009 at 08:31:15AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > ---
> >  drivers/usb/class/cdc-acm.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> > index 38bfdb0..02eb60b 100644
> > --- a/drivers/usb/class/cdc-acm.c
> > +++ b/drivers/usb/class/cdc-acm.c
> > @@ -550,7 +550,7 @@ static void acm_waker(struct work_struct *waker)
> >  static int acm_tty_open(struct tty_struct *tty, struct file *filp)
> >  {
> >  	struct acm *acm;
> > -	int rv = -EINVAL;
> > +	int rv = -ENODEV;
>
> Why is this needed?

The current error return is incorrect. If the table entry is gone, the
device has been disconnected, hence -ENODEV.

	Regards
		Oliver
Acked-by: Oliver Neukum <oliver@neukum.org>

Re: [PATCH 1/3] Return ENODEV instead of EINVAL when trying to open ACM device.

[Thadeu Lima de Souza Cascardo]

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

On Wed, Jun 24, 2009 at 11:17:04PM +0200, Oliver Neukum wrote:
> Am Mittwoch, 24. Juni 2009 22:40:15 schrieb Greg KH:
> > On Wed, Jun 24, 2009 at 08:31:15AM -0300, Thadeu Lima de Souza Cascardo wrote:
> > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > > ---
> > >  drivers/usb/class/cdc-acm.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> > > index 38bfdb0..02eb60b 100644
> > > --- a/drivers/usb/class/cdc-acm.c
> > > +++ b/drivers/usb/class/cdc-acm.c
> > > @@ -550,7 +550,7 @@ static void acm_waker(struct work_struct *waker)
> > >  static int acm_tty_open(struct tty_struct *tty, struct file *filp)
> > >  {
> > >  	struct acm *acm;
> > > -	int rv = -EINVAL;
> > > +	int rv = -ENODEV;
> >
> > Why is this needed?
> 
> The current error return is incorrect. If the table entry is gone, the
> device has been disconnected, hence -ENODEV.
> 
> 	Regards
> 		Oliver
> Acked-by: Oliver Neukum <oliver@neukum.org>
> 

Or if the device has never been connected at all.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH 1/3] Return ENODEV instead of EINVAL when trying to open ACM device.

[Thadeu Lima de Souza Cascardo]

This is required, otherwise a user will get a EINVAL while opening a
non-existing device, instead of ENODEV.

This is what I get with this patch applied now instead of an "Invalid
argument".

cascardo@vespa:~$ cat /dev/ttyACM0
cat: /dev/ttyACM0: No such device
cascardo@vespa:~$

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/usb/class/cdc-acm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 38bfdb0..02eb60b 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -550,7 +550,7 @@ static void acm_waker(struct work_struct *waker)
 static int acm_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct acm *acm;
-	int rv = -EINVAL;
+	int rv = -ENODEV;
 	int i;
 	dbg("Entering acm_tty_open.");
 
-- 
1.6.3