From 41d37b860809ad5d2784ab444c4d392c9dd88b0b Mon Sep 17 00:00:00 2001 From: Tomohiro Kusumi Date: Wed, 11 Nov 2015 19:02:10 +0900 Subject: [PATCH] sys/dev/disk/dm: Fix/refactor alloc/free functions [3/6] dm_dev_alloc()/free() * Make them static. * Make alloc() take name and uuid just like dm_pdev_alloc(). * Refactoring. dm_pdev_alloc()/free() * Fix memory leak. * Rename dm_pdev_rem() to dm_pdev_free(). A simple kfree function should be xxx_free() but not xxx_rem(). xxx_rem() isn't necessarily a name used for just kfree() in other files, which makes naming very confusing. * Refactoring. --- sys/dev/disk/dm/dm.h | 2 -- sys/dev/disk/dm/dm_dev.c | 34 +++++++++++++++++++--------------- sys/dev/disk/dm/dm_pdev.c | 25 +++++++++++++------------ 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/sys/dev/disk/dm/dm.h b/sys/dev/disk/dm/dm.h index e27de3d909..491881cf7e 100644 --- a/sys/dev/disk/dm/dm.h +++ b/sys/dev/disk/dm/dm.h @@ -260,12 +260,10 @@ void dm_table_free_deps(dm_table_entry_t *table_en); /* dm_dev.c */ int dm_dev_init(void); int dm_dev_uninit(void); -dm_dev_t* dm_dev_alloc(void); void dm_dev_busy(dm_dev_t *); int dm_dev_create(dm_dev_t **, const char *, const char *, int); int dm_dev_remove(dm_dev_t *); int dm_dev_remove_all(int); -int dm_dev_free(dm_dev_t *); int dm_dev_insert(dm_dev_t *); dm_dev_t* dm_dev_lookup(const char *, const char *, int); prop_array_t dm_dev_prop_list(void); diff --git a/sys/dev/disk/dm/dm_dev.c b/sys/dev/disk/dm/dm_dev.c index f235940181..37829a0a50 100644 --- a/sys/dev/disk/dm/dm_dev.c +++ b/sys/dev/disk/dm/dm_dev.c @@ -54,6 +54,8 @@ static dm_dev_t *dm_dev_lookup_name(const char *); static dm_dev_t *dm_dev_lookup_uuid(const char *); static dm_dev_t *dm_dev_lookup_minor(int); static int dm_dev_destroy(dm_dev_t *); +static dm_dev_t *dm_dev_alloc(const char *, const char *); +static int dm_dev_free(dm_dev_t *); static TAILQ_HEAD(dm_dev_head, dm_dev) dm_dev_list; @@ -223,17 +225,9 @@ dm_dev_create(dm_dev_t **dmvp, const char *name, const char *uuid, int flags) char name_buf[MAXPATHLEN]; int r, dm_minor; - if ((dmv = dm_dev_alloc()) == NULL) + if ((dmv = dm_dev_alloc(name, uuid)) == NULL) return ENOMEM; - if (uuid) - strncpy(dmv->uuid, uuid, DM_UUID_LEN); - else - dmv->uuid[0] = '\0'; - - if (name) - strlcpy(dmv->name, name, DM_NAME_LEN); - dm_minor = devfs_clone_bitmap_get(&dm_minor_bitmap, 0); dm_table_head_init(&dmv->table_head); @@ -355,22 +349,32 @@ dm_dev_remove_all(int gentle) /* * Allocate new device entry. */ -dm_dev_t * -dm_dev_alloc(void) +static dm_dev_t * +dm_dev_alloc(const char *name, const char*uuid) { dm_dev_t *dmv; - dmv = kmalloc(sizeof(dm_dev_t), M_DM, M_WAITOK | M_ZERO); + dmv = kmalloc(sizeof(*dmv), M_DM, M_WAITOK | M_ZERO); + if (dmv == NULL) + return NULL; - if (dmv != NULL) - dmv->diskp = kmalloc(sizeof(struct disk), M_DM, M_WAITOK | M_ZERO); + dmv->diskp = kmalloc(sizeof(*dmv->diskp), M_DM, M_WAITOK | M_ZERO); + if (dmv->diskp == NULL) { + kfree(dmv, M_DM); + return NULL; + } + + if (name) + strlcpy(dmv->name, name, sizeof(dmv->name)); + if (uuid) + strncpy(dmv->uuid, uuid, sizeof(dmv->uuid)); return dmv; } /* * Freed device entry. */ -int +static int dm_dev_free(dm_dev_t *dmv) { KKASSERT(dmv != NULL); diff --git a/sys/dev/disk/dm/dm_pdev.c b/sys/dev/disk/dm/dm_pdev.c index a6af07ddd2..6898ab2f39 100644 --- a/sys/dev/disk/dm/dm_pdev.c +++ b/sys/dev/disk/dm/dm_pdev.c @@ -46,7 +46,7 @@ static TAILQ_HEAD(, dm_pdev) dm_pdev_list; static struct lock dm_pdev_mutex; static dm_pdev_t *dm_pdev_alloc(const char *); -static int dm_pdev_rem(dm_pdev_t *); +static int dm_pdev_free(dm_pdev_t *); static dm_pdev_t *dm_pdev_lookup_name(const char *); /* @@ -147,7 +147,7 @@ dm_pdev_insert(const char *dev_name) if (error) { dmdebug("dk_lookup on device: %s failed with error %d!\n", dev_name, error); - dm_pdev_rem(dmp); + dm_pdev_free(dmp); lockmgr(&dm_pdev_mutex, LK_RELEASE); return NULL; } @@ -155,7 +155,7 @@ dm_pdev_insert(const char *dev_name) if (dm_pdev_get_vattr(dmp, &va) == -1) { dmdebug("makeudev %s failed\n", dev_name); - dm_pdev_rem(dmp); + dm_pdev_free(dmp); lockmgr(&dm_pdev_mutex, LK_RELEASE); return NULL; } @@ -200,13 +200,12 @@ dm_pdev_alloc(const char *name) { dm_pdev_t *dmp; - if ((dmp = kmalloc(sizeof(dm_pdev_t), M_DM, M_WAITOK | M_ZERO)) == NULL) + dmp = kmalloc(sizeof(*dmp), M_DM, M_WAITOK | M_ZERO); + if (dmp == NULL) return NULL; - strlcpy(dmp->name, name, MAX_DEV_NAME); - - dmp->ref_cnt = 0; - dmp->pdev_vnode = NULL; + if (name) + strlcpy(dmp->name, name, MAX_DEV_NAME); return dmp; } @@ -214,7 +213,7 @@ dm_pdev_alloc(const char *name) * Destroy allocated dm_pdev. */ static int -dm_pdev_rem(dm_pdev_t *dmp) +dm_pdev_free(dm_pdev_t *dmp) { int err; @@ -222,8 +221,10 @@ dm_pdev_rem(dm_pdev_t *dmp) if (dmp->pdev_vnode != NULL) { err = vn_close(dmp->pdev_vnode, FREAD | FWRITE, NULL); - if (err != 0) + if (err != 0) { + kfree(dmp, M_DM); return err; + } } kfree(dmp, M_DM); @@ -254,7 +255,7 @@ dm_pdev_decr(dm_pdev_t *dmp) if (--dmp->ref_cnt == 0) { TAILQ_REMOVE(&dm_pdev_list, dmp, next_pdev); lockmgr(&dm_pdev_mutex, LK_RELEASE); - dm_pdev_rem(dmp); + dm_pdev_free(dmp); return 0; } lockmgr(&dm_pdev_mutex, LK_RELEASE); @@ -319,7 +320,7 @@ dm_pdev_uninit(void) while ((dmp = TAILQ_FIRST(&dm_pdev_list)) != NULL) { TAILQ_REMOVE(&dm_pdev_list, dmp, next_pdev); - dm_pdev_rem(dmp); + dm_pdev_free(dmp); } KKASSERT(TAILQ_EMPTY(&dm_pdev_list)); -- 2.41.0