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/