2005-05-07 00:55:30

by Jesper Juhl

[permalink] [raw]
Subject: [PATCH] kfree cleanups (remove redundant NULL checks) in drivers/telephony/ (actually ixj.c only)

This patch removes redundant checks for NULL pointer before kfree() in
drivers/telephony/


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

drivers/telephony/ixj.c | 48 +++++++++++++++++-------------------------------
1 files changed, 17 insertions(+), 31 deletions(-)

diff -upr linux-2.6.12-rc3-mm3-orig/drivers/telephony/ixj.c linux-2.6.12-rc3-mm3/drivers/telephony/ixj.c
--- linux-2.6.12-rc3-mm3-orig/drivers/telephony/ixj.c 2005-05-06 23:21:17.000000000 +0200
+++ linux-2.6.12-rc3-mm3/drivers/telephony/ixj.c 2005-05-07 02:53:15.000000000 +0200
@@ -329,10 +329,8 @@ static IXJ *ixj_alloc()

static void ixj_fsk_free(IXJ *j)
{
- if(j->fskdata != NULL) {
- kfree(j->fskdata);
- j->fskdata = NULL;
- }
+ kfree(j->fskdata);
+ j->fskdata = NULL;
}

static void ixj_fsk_alloc(IXJ *j)
@@ -3869,11 +3867,9 @@ static int set_rec_codec(IXJ *j, int rat
default:
j->rec_frame_size = 0;
j->rec_mode = -1;
- if (j->read_buffer) {
- kfree(j->read_buffer);
- j->read_buffer = NULL;
- j->read_buffer_size = 0;
- }
+ kfree(j->read_buffer);
+ j->read_buffer = NULL;
+ j->read_buffer_size = 0;
retval = 1;
break;
}
@@ -3994,11 +3990,9 @@ static void ixj_record_stop(IXJ *j)
if(ixjdebug & 0x0002)
printk("IXJ %d Stopping Record Codec %d at %ld\n", j->board, j->rec_codec, jiffies);

- if (j->read_buffer) {
- kfree(j->read_buffer);
- j->read_buffer = NULL;
- j->read_buffer_size = 0;
- }
+ kfree(j->read_buffer);
+ j->read_buffer = NULL;
+ j->read_buffer_size = 0;
if (j->rec_mode > -1) {
ixj_WriteDSPCommand(0x5120, j);
j->rec_mode = -1;
@@ -4451,11 +4445,9 @@ static int set_play_codec(IXJ *j, int ra
default:
j->play_frame_size = 0;
j->play_mode = -1;
- if (j->write_buffer) {
- kfree(j->write_buffer);
- j->write_buffer = NULL;
- j->write_buffer_size = 0;
- }
+ kfree(j->write_buffer);
+ j->write_buffer = NULL;
+ j->write_buffer_size = 0;
retval = 1;
break;
}
@@ -4581,11 +4573,9 @@ static void ixj_play_stop(IXJ *j)
if(ixjdebug & 0x0002)
printk("IXJ %d Stopping Play Codec %d at %ld\n", j->board, j->play_codec, jiffies);

- if (j->write_buffer) {
- kfree(j->write_buffer);
- j->write_buffer = NULL;
- j->write_buffer_size = 0;
- }
+ kfree(j->write_buffer);
+ j->write_buffer = NULL;
+ j->write_buffer_size = 0;
if (j->play_mode > -1) {
ixj_WriteDSPCommand(0x5221, j); /* Stop playback and flush buffers. 8022 reference page 9-40 */

@@ -5810,9 +5800,7 @@ static void ixj_cpt_stop(IXJ *j)
ixj_play_tone(j, 0);
j->tone_state = j->tone_cadence_state = 0;
if (j->cadence_t) {
- if (j->cadence_t->ce) {
- kfree(j->cadence_t->ce);
- }
+ kfree(j->cadence_t->ce);
kfree(j->cadence_t);
j->cadence_t = NULL;
}
@@ -7497,10 +7485,8 @@ static void cleanup(void)
printk(KERN_INFO "IXJ: Releasing XILINX address for /dev/phone%d\n", cnt);
release_region(j->XILINXbase, 4);
}
- if (j->read_buffer)
- kfree(j->read_buffer);
- if (j->write_buffer)
- kfree(j->write_buffer);
+ kfree(j->read_buffer);
+ kfree(j->write_buffer);
if (j->dev)
pnp_device_detach(j->dev);
if (ixjdebug & 0x0002)




2005-05-07 01:51:07

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH] kfree cleanups (remove redundant NULL checks) in drivers/telephony/ (actually ixj.c only)

On Sat, 7 May 2005, Jesper Juhl wrote:

> This patch removes redundant checks for NULL pointer before kfree() in
> drivers/telephony/
>
Joe Perches pointed out to me that
kfree
followed by
setting all structure members would be slightly better.

Incremental patch below.


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

drivers/telephony/ixj.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

--- linux-2.6.12-rc3-mm3/drivers/telephony/ixj.c.old 2005-05-07 03:50:43.000000000 +0200
+++ linux-2.6.12-rc3-mm3/drivers/telephony/ixj.c 2005-05-07 03:53:22.000000000 +0200
@@ -3865,9 +3865,9 @@ static int set_rec_codec(IXJ *j, int rat
j->rec_mode = 7;
break;
default:
+ kfree(j->read_buffer);
j->rec_frame_size = 0;
j->rec_mode = -1;
- kfree(j->read_buffer);
j->read_buffer = NULL;
j->read_buffer_size = 0;
retval = 1;
@@ -3987,7 +3987,7 @@ static int ixj_record_start(IXJ *j)

static void ixj_record_stop(IXJ *j)
{
- if(ixjdebug & 0x0002)
+ if (ixjdebug & 0x0002)
printk("IXJ %d Stopping Record Codec %d at %ld\n", j->board, j->rec_codec, jiffies);

kfree(j->read_buffer);
@@ -4443,9 +4443,9 @@ static int set_play_codec(IXJ *j, int ra
j->play_mode = 5;
break;
default:
+ kfree(j->write_buffer);
j->play_frame_size = 0;
j->play_mode = -1;
- kfree(j->write_buffer);
j->write_buffer = NULL;
j->write_buffer_size = 0;
retval = 1;
@@ -4570,7 +4570,7 @@ static int ixj_play_start(IXJ *j)

static void ixj_play_stop(IXJ *j)
{
- if(ixjdebug & 0x0002)
+ if (ixjdebug & 0x0002)
printk("IXJ %d Stopping Play Codec %d at %ld\n", j->board, j->play_codec, jiffies);

kfree(j->write_buffer);


2005-05-07 02:03:27

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] kfree cleanups (remove redundant NULL checks) in drivers/telephony/ (actually ixj.c only)


This patch adds behavioural changes:

- if (j->read_buffer) {
- kfree(j->read_buffer);
- j->read_buffer = NULL;
- j->read_buffer_size = 0;
- }
+ j->read_buffer = NULL;
+ j->read_buffer_size = 0;

Now we'll zero ->read_buffer_size even if ->read_buffer was already NULL.

It's hard to believe that this could cause any problems, but please check
that.

2005-05-07 02:06:17

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH] kfree cleanups (remove redundant NULL checks) in drivers/telephony/ (actually ixj.c only)

On Fri, 6 May 2005, Andrew Morton wrote:

>
> This patch adds behavioural changes:
>
> - if (j->read_buffer) {
> - kfree(j->read_buffer);
> - j->read_buffer = NULL;
> - j->read_buffer_size = 0;
> - }
> + j->read_buffer = NULL;
> + j->read_buffer_size = 0;
>
> Now we'll zero ->read_buffer_size even if ->read_buffer was already NULL.
>
> It's hard to believe that this could cause any problems, but please check
> that.
>
When I initially read the code I didn't see any harm that could be done by
that, but I'll take a second more careful look and report back - I'm
pretty sure it's safe though.
I'll do this tomorrow since I'm off to catch some sleep now.

--
Jesper

2005-05-08 21:59:19

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH] kfree cleanups (remove redundant NULL checks) in drivers/telephony/ (actually ixj.c only)

On Sat, 7 May 2005, Jesper Juhl wrote:

> On Fri, 6 May 2005, Andrew Morton wrote:
>
> >
> > This patch adds behavioural changes:
> >
[snip]
> > Now we'll zero ->read_buffer_size even if ->read_buffer was already NULL.
> >
> > It's hard to believe that this could cause any problems, but please check
> > that.
> >
> When I initially read the code I didn't see any harm that could be done by
> that, but I'll take a second more careful look and report back - I'm
> pretty sure it's safe though.
> I'll do this tomorrow since I'm off to catch some sleep now.
>
Ok, I've taken a second look at the code, and I don't see any places where
read_buffer_size are used where read_buffer is NULL, so zeroing
read_buffer_size unconditionally when kfree()'ing read_buffer should be
quite safe.


--
Jesper