On 21/07/07, Lars Ellenberg <[email protected]> wrote:
DRBD wants to go mainline.
please have a look at the "for-linus" branch of
git://git.drbd.org/home/git/linux-drbd.git.
A few more comments.
In drbd_main.c::after_state_ch() there's this code :
...
if ( mdev->p_uuid ) {
kfree(mdev->p_uuid);
mdev->p_uuid = NULL;
}
...
Since kfree(NULL) if a noop, that should just become :
...
kfree(mdev->p_uuid);
mdev->p_uuid = NULL;
...
Same for these bits in drbd_main.c::drbd_cleanup()
...
if(mdev->app_reads_hash) {
kfree(mdev->app_reads_hash);
mdev->app_reads_hash = NULL;
}
if ( mdev->p_uuid ) {
kfree(mdev->p_uuid);
mdev->p_uuid = NULL;
}
...
They should just become
...
kfree(mdev->app_reads_hash);
mdev->app_reads_hash = NULL;
kfree(mdev->p_uuid);
mdev->p_uuid = NULL;
...
And then there's drbd_main.c::drbd_new_device() :
...
if(mdev->app_reads_hash) kfree(mdev->app_reads_hash);
...
That should just be
...
kfree(mdev->app_reads_hash);
...
Btw; You shouldn't put multiple statements on the same line. That is
seen all over the place - like
mdev = kzalloc(sizeof(drbd_dev),GFP_KERNEL);
if(!mdev) goto Enomem;
That should be
mdev = kzalloc(sizeof(drbd_dev),GFP_KERNEL);
if(!mdev)
goto Enomem;
There are lots of other examples.
Here are a few more pointless NULL checks before kfree() :
drbd_nl.c::drbd_nl_net_conf() :
...
if(new_tl_hash) {
if (mdev->tl_hash) kfree(mdev->tl_hash);
mdev->tl_hash_s = mdev->net_conf->max_epoch_size/8;
mdev->tl_hash = new_tl_hash;
}
if(new_ee_hash) {
if (mdev->ee_hash) kfree(mdev->ee_hash);
mdev->ee_hash_s = mdev->net_conf->max_buffers/8;
mdev->ee_hash = new_ee_hash;
}
if ( mdev->cram_hmac_tfm ) {
crypto_free_hash(mdev->cram_hmac_tfm);
}
mdev->cram_hmac_tfm = tfm;
...
should become :
...
if(new_tl_hash) {
kfree(mdev->tl_hash);
mdev->tl_hash_s = mdev->net_conf->max_epoch_size/8;
mdev->tl_hash = new_tl_hash;
}
if(new_ee_hash) {
kfree(mdev->ee_hash);
mdev->ee_hash_s = mdev->net_conf->max_buffers/8;
mdev->ee_hash = new_ee_hash;
}
crypto_free_hash(mdev->cram_hmac_tfm);
mdev->cram_hmac_tfm = tfm;
...
still in drbd_nl.c::drbd_nl_net_conf() :
...
fail:
if (tfm) crypto_free_hash(tfm);
if (new_tl_hash) kfree(new_tl_hash);
if (new_ee_hash) kfree(new_ee_hash);
if (new_conf) kfree(new_conf);
...
should become
...
fail:
crypto_free_hash(tfm);
kfree(new_tl_hash);
kfree(new_ee_hash);
kfree(new_conf);
...
drbd_nl.c::ensure_mdev() :
...
if(mdev->app_reads_hash) kfree(mdev->app_reads_hash);
..
becomes
...
kfree(mdev->app_reads_hash);
...
In drbd_receiver.c::receive_uuids() we find this :
...
if ( mdev->p_uuid ) kfree(mdev->p_uuid);
...
which should be just
...
kfree(mdev->p_uuid);
...
There is also drbd_receiver.c::drbd_disconnect() - where
...
if ( mdev->p_uuid ) {
kfree(mdev->p_uuid);
mdev->p_uuid = NULL;
}
...
needs to become just
...
kfree(mdev->p_uuid);
mdev->p_uuid = NULL;
...
And in drbd_receiver.c::drbd_do_auth()
...
fail:
if(peers_ch) kfree(peers_ch);
if(response) kfree(response);
if(right_response) kfree(right_response);
...
needs to be turned into
...
fail:
kfree(peers_ch);
kfree(response);
kfree(right_response);
...
drbd_req.c::drbd_make_request_common() also needs to get rid of
...
if (b) kfree(b); /* if someone else has beaten us to it... */
...
fail_and_free_req:
if (b) kfree(b);
...
and just turn it into
...
kfree(b); /* if someone else has beaten us to it... */
...
fail_and_free_req:
kfree(b);
...
--
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
