2006-05-15 21:56:42

by Jesper Juhl

[permalink] [raw]
Subject: [PATCH 2/3] moxa: remove pointless check of 'tty' argument vs NULL


Remove pointless check of 'tty' argument vs NULL from moxa driver.

(applies on top of patch 1/3)


Signed-off-by: Jesper Juhl <[email protected]>
---

drivers/char/mxser.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

--- linux-2.6.17-rc4-mm1/drivers/char/mxser.c.1 2006-05-15 22:33:49.000000000 +0200
+++ linux-2.6.17-rc4-mm1/drivers/char/mxser.c 2006-05-15 22:34:38.000000000 +0200
@@ -1081,7 +1081,7 @@ static int mxser_write(struct tty_struct
struct mxser_struct *info = tty->driver_data;
unsigned long flags;

- if (!tty || !info->xmit_buf)
+ if (!info->xmit_buf)
return (0);

while (1) {
@@ -1117,7 +1117,7 @@ static void mxser_put_char(struct tty_st
struct mxser_struct *info = tty->driver_data;
unsigned long flags;

- if (!tty || !info->xmit_buf)
+ if (!info->xmit_buf)
return;

if (info->xmit_cnt >= SERIAL_XMIT_SIZE - 1)




2006-05-15 21:59:12

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 2/3] moxa: remove pointless check of 'tty' argument vs NULL

On Mon, May 15, 2006 at 11:57:35PM +0200, Jesper Juhl wrote:
> Remove pointless check of 'tty' argument vs NULL from moxa driver.

Can you leave those in for the moment but change them to BUG_ON() because
I've seen the pop up once or twice. They may be fixed but Im not 100% sure
yet.

> + if (!info->xmit_buf)
> return (0);

2006-05-15 22:08:43

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH 2/3] moxa: remove pointless check of 'tty' argument vs NULL

On 15/05/06, Alan Cox <[email protected]> wrote:
> On Mon, May 15, 2006 at 11:57:35PM +0200, Jesper Juhl wrote:
> > Remove pointless check of 'tty' argument vs NULL from moxa driver.
>
> Can you leave those in for the moment but change them to BUG_ON() because
> I've seen the pop up once or twice. They may be fixed but Im not 100% sure
> yet.
>

I could, but would that really make sense?

As Alexey pointed out in the previous thread :

> ->write() is called via
>
> tty->driver->write(tty, ...);
>
> See? tty was already dereferenced.

And besides, we wouldn't ever get to the BUG_ON() because even if the
function did manage to get called it would then explode in the
struct mxser_struct *info = tty->driver_data;
assignment, before it ever got to the BUG_ON() (that assignment could
ofcourse be moved as I did in my original patch, but that still leaves
Alexeys point in place).

--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

2006-05-15 22:10:22

by Alexey Dobriyan

[permalink] [raw]
Subject: Re: [PATCH 2/3] moxa: remove pointless check of 'tty' argument vs NULL

On Mon, May 15, 2006 at 05:59:01PM -0400, Alan Cox wrote:
> On Mon, May 15, 2006 at 11:57:35PM +0200, Jesper Juhl wrote:
> > Remove pointless check of 'tty' argument vs NULL from moxa driver.
>
> Can you leave those in for the moment but change them to BUG_ON() because
> I've seen the pop up once or twice. They may be fixed but Im not 100% sure
> yet.

Dereference to get ->driver_data will show the very same backtrace?

struct mxser_struct *info = (struct mxser_struct *) tty->driver_data;

- if (!tty || !info->xmit_buf)
> > + if (!info->xmit_buf)
> > return (0);

2006-05-15 22:50:05

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 2/3] moxa: remove pointless check of 'tty' argument vs NULL

On Tue, May 16, 2006 at 12:08:41AM +0200, Jesper Juhl wrote:
> > See? tty was already dereferenced.
>
> And besides, we wouldn't ever get to the BUG_ON() because even if the
> function did manage to get called it would then explode in the

Agreed - your analysis is right