From 07270bcff85407c6fa4da3c151302837318816e3 Mon Sep 17 00:00:00 2001 From: Faruk Kasumovic Date: Thu, 23 Feb 2017 11:29:17 +0100 Subject: [PATCH] - Improved pointer handling between go and C and elimination of associated memory leaks and potential corruptions --- common.go | 2 +- zfs.c | 14 ++++++++-- zfs.go | 46 ++++++++++++++++++-------------- zfs.h | 2 ++ zpool.go | 78 +++++++++++++++++++++++++++---------------------------- zpool.h | 9 +++++++ 6 files changed, 90 insertions(+), 61 deletions(-) diff --git a/common.go b/common.go index 002a2b5..2cffd35 100644 --- a/common.go +++ b/common.go @@ -26,7 +26,7 @@ import ( // VDevType type of device in the pool type VDevType string -var libzfsHandle *C.struct_libzfs_handle +var libzfsHandle C.libzfs_handle_ptr func init() { libzfsHandle = C.libzfs_init() diff --git a/zfs.c b/zfs.c index f079ce6..9089392 100644 --- a/zfs.c +++ b/zfs.c @@ -20,6 +20,16 @@ dataset_list_t *create_dataset_list_item() { void dataset_list_close(dataset_list_t *list) { zfs_close(list->zh); free(list); + // dataset_list_free(list); +} + +void dataset_list_free(dataset_list_t *list) { + dataset_list_t *next; + while(list) { + next = list->pnext; + free(list); + list = next; + } } int dataset_list_callb(zfs_handle_t *dataset, void *data) { @@ -44,7 +54,7 @@ int dataset_list_root(libzfs_handle_t *libzfs, dataset_list_t **first) { *first = zlist; } else { *first = 0; - free(zlist); + dataset_list_free(zlist); } return err; } @@ -62,7 +72,7 @@ int dataset_list_children(zfs_handle_t *zfs, dataset_list_t **first) { *first = zlist; } else { *first = 0; - free(zlist); + dataset_list_free(zlist); } return err; } diff --git a/zfs.go b/zfs.go index e5a94b4..10a69f1 100644 --- a/zfs.go +++ b/zfs.go @@ -8,6 +8,7 @@ import "C" import ( "errors" + "fmt" "unsafe" ) @@ -36,17 +37,18 @@ const ( // Dataset - ZFS dataset object type Dataset struct { - list *C.dataset_list_t + list C.dataset_list_ptr Type DatasetType Properties map[Prop]Property Children []Dataset } func (d *Dataset) openChildren() (err error) { - var dataset Dataset + var list C.dataset_list_ptr d.Children = make([]Dataset, 0, 5) - errcode := C.dataset_list_children(d.list.zh, &(dataset.list)) - for dataset.list != nil { + errcode := C.dataset_list_children(d.list.zh, unsafe.Pointer(&list)) + for list != nil { + dataset := Dataset{list: list} dataset.Type = DatasetType(C.zfs_get_type(dataset.list.zh)) dataset.Properties = make(map[Prop]Property) err = dataset.ReloadProperties() @@ -54,7 +56,7 @@ func (d *Dataset) openChildren() (err error) { return } d.Children = append(d.Children, dataset) - dataset.list = C.dataset_next(dataset.list) + list = C.dataset_next(list) } if errcode != 0 { err = LastError() @@ -72,7 +74,7 @@ func (d *Dataset) openChildren() (err error) { // (file-systems, volumes or snapshots). func DatasetOpenAll() (datasets []Dataset, err error) { var dataset Dataset - errcode := C.dataset_list_root(libzfsHandle, &dataset.list) + errcode := C.dataset_list_root(libzfsHandle, unsafe.Pointer(&dataset.list)) for dataset.list != nil { dataset.Type = DatasetType(C.zfs_get_type(dataset.list.zh)) err = dataset.ReloadProperties() @@ -111,6 +113,9 @@ func DatasetOpen(path string) (d Dataset, err error) { if d.list.zh == nil { err = LastError() + if err == nil { + err = fmt.Errorf("dataset not found.") + } return } d.Type = DatasetType(C.zfs_get_type(d.list.zh)) @@ -124,9 +129,9 @@ func DatasetOpen(path string) (d Dataset, err error) { } func datasetPropertiesTonvlist(props map[Prop]Property) ( - cprops *C.nvlist_t, err error) { + cprops C.nvlist_ptr, err error) { // convert properties to nvlist C type - r := C.nvlist_alloc(&cprops, C.NV_UNIQUE_NAME, 0) + r := C.nvlist_alloc(unsafe.Pointer(&cprops), C.NV_UNIQUE_NAME, 0) if r != 0 { err = errors.New("Failed to allocate properties") return @@ -149,7 +154,7 @@ func datasetPropertiesTonvlist(props map[Prop]Property) ( // pool/dataset or pool/parent/dataset func DatasetCreate(path string, dtype DatasetType, props map[Prop]Property) (d Dataset, err error) { - var cprops *C.nvlist_t + var cprops C.nvlist_ptr if cprops, err = datasetPropertiesTonvlist(props); err != nil { return } @@ -170,6 +175,7 @@ func DatasetCreate(path string, dtype DatasetType, func (d *Dataset) Close() { if d.list != nil && d.list.zh != nil { C.dataset_list_close(d.list) + d.list = nil } for _, cd := range d.Children { cd.Close() @@ -242,17 +248,19 @@ func (d *Dataset) ReloadProperties() (err error) { err = errors.New(msgDatasetIsNil) return } - var plist *C.property_list_t - plist = C.new_property_list() - defer C.free_properties(plist) + var plist C.property_list_ptr + d.Properties = make(map[Prop]Property) for prop := DatasetPropType; prop < DatasetNumProps; prop++ { + plist = C.new_property_list() errcode := C.read_dataset_property(d.list.zh, plist, C.int(prop)) if errcode != 0 { + C.free_properties(plist) continue } d.Properties[prop] = Property{Value: C.GoString(&(*plist).value[0]), Source: C.GoString(&(*plist).source[0])} + C.free_properties(plist) } return } @@ -264,7 +272,7 @@ func (d *Dataset) GetProperty(p Prop) (prop Property, err error) { err = errors.New(msgDatasetIsNil) return } - var plist *C.property_list_t + var plist C.property_list_ptr plist = C.new_property_list() defer C.free_properties(plist) errcode := C.read_dataset_property(d.list.zh, plist, C.int(p)) @@ -283,7 +291,7 @@ func (d *Dataset) GetUserProperty(p string) (prop Property, err error) { err = errors.New(msgDatasetIsNil) return } - var plist *C.property_list_t + var plist C.property_list_ptr plist = C.new_property_list() defer C.free_properties(plist) csp := C.CString(p) @@ -339,7 +347,7 @@ func (d *Dataset) SetUserProperty(prop, value string) (err error) { // Clone - clones the dataset. The target must be of the same type as // the source. func (d *Dataset) Clone(target string, props map[Prop]Property) (rd Dataset, err error) { - var cprops *C.nvlist_t + var cprops C.nvlist_ptr if d.list == nil { err = errors.New(msgDatasetIsNil) return @@ -360,7 +368,7 @@ func (d *Dataset) Clone(target string, props map[Prop]Property) (rd Dataset, err // DatasetSnapshot create dataset snapshot. Set recur to true to snapshot child datasets. func DatasetSnapshot(path string, recur bool, props map[Prop]Property) (rd Dataset, err error) { - var cprops *C.nvlist_t + var cprops C.nvlist_ptr if cprops, err = datasetPropertiesTonvlist(props); err != nil { return } @@ -419,12 +427,12 @@ func (d *Dataset) Rename(newName string, recur, // sets in 'where' argument the current mountpoint, and returns true. Otherwise, // returns false. func (d *Dataset) IsMounted() (mounted bool, where string) { - var cw *C.char + var cw C.char_ptr if d.list == nil { return false, "" } - m := C.zfs_is_mounted(d.list.zh, &cw) - defer C.free(unsafe.Pointer(cw)) + m := C.zfs_is_mounted(d.list.zh, unsafe.Pointer(&cw)) + // defer C.free(cw) if m != 0 { return true, C.GoString(cw) } diff --git a/zfs.h b/zfs.h index 6497961..0885e25 100644 --- a/zfs.h +++ b/zfs.h @@ -11,10 +11,12 @@ struct dataset_list { }; typedef struct dataset_list dataset_list_t; +typedef struct dataset_list* dataset_list_ptr; dataset_list_t *create_dataset_list_item(); void dataset_list_close(dataset_list_t *list); +void dataset_list_free(dataset_list_t *list); int dataset_list_root(libzfs_handle_t *libzfs, dataset_list_t **first); int dataset_list_children(zfs_handle_t *zfs, dataset_list_t **first); diff --git a/zpool.go b/zpool.go index df9b203..a8dfc6b 100644 --- a/zpool.go +++ b/zpool.go @@ -127,7 +127,7 @@ type ExportedPool struct { // give easy access to listing all available properties. It can be refreshed // with up to date values with call to (*Pool) ReloadProperties type Pool struct { - list *C.zpool_list_t + list C.zpool_list_ptr Properties []Property Features map[string]string } @@ -148,14 +148,14 @@ func PoolOpen(name string) (pool Pool, err error) { return } -func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { - var dtype *C.char +func poolGetConfig(name string, nv C.nvlist_ptr) (vdevs VDevTree, err error) { + var dtype C.char_ptr var c, children C.uint_t var notpresent C.uint64_t - var vs *C.vdev_stat_t - var ps *C.pool_scan_stat_t - var child **C.nvlist_t - if 0 != C.nvlist_lookup_string(nv, C.sZPOOL_CONFIG_TYPE, &dtype) { + var vs C.vdev_stat_ptr + var ps C.pool_scan_stat_ptr + var child *C.nvlist_ptr + if 0 != C.nvlist_lookup_string(nv, C.sZPOOL_CONFIG_TYPE, unsafe.Pointer(&dtype)) { err = fmt.Errorf("Failed to fetch %s", C.ZPOOL_CONFIG_TYPE) return } @@ -167,7 +167,7 @@ func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { // Fetch vdev state if 0 != C.nvlist_lookup_uint64_array_vds(nv, C.sZPOOL_CONFIG_VDEV_STATS, - &vs, &c) { + unsafe.Pointer(&vs), &c) { err = fmt.Errorf("Failed to fetch %s", C.ZPOOL_CONFIG_VDEV_STATS) return } @@ -193,7 +193,7 @@ func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { // Fetch vdev scan stats if 0 == C.nvlist_lookup_uint64_array_ps(nv, C.sZPOOL_CONFIG_SCAN_STATS, - &ps, &c) { + unsafe.Pointer(&ps), &c) { vdevs.ScanStat.Func = uint64(ps.pss_func) vdevs.ScanStat.State = uint64(ps.pss_state) vdevs.ScanStat.StartTime = uint64(ps.pss_start_time) @@ -209,7 +209,7 @@ func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { // Fetch the children if C.nvlist_lookup_nvlist_array(nv, C.sZPOOL_CONFIG_CHILDREN, - &child, &children) != 0 { + unsafe.Pointer(&child), &children) != 0 { return } if children > 0 { @@ -217,8 +217,8 @@ func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { } if C.nvlist_lookup_uint64(nv, C.sZPOOL_CONFIG_NOT_PRESENT, ¬present) == 0 { - var path *C.char - if 0 != C.nvlist_lookup_string(nv, C.sZPOOL_CONFIG_PATH, &path) { + var path C.char_ptr + if 0 != C.nvlist_lookup_string(nv, C.sZPOOL_CONFIG_PATH, unsafe.Pointer(&path)) { err = fmt.Errorf("Failed to fetch %s", C.ZPOOL_CONFIG_PATH) return } @@ -249,13 +249,13 @@ func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { // PoolImportSearch - Search pools available to import but not imported. // Returns array of found pools. func PoolImportSearch(searchpaths []string) (epools []ExportedPool, err error) { - var config, nvroot *C.nvlist_t - var cname, msgid, comment *C.char + var config, nvroot C.nvlist_ptr + var cname, msgid, comment C.char_ptr var poolState, guid C.uint64_t var reason C.zpool_status_t var errata C.zpool_errata_t config = nil - var elem *C.nvpair_t + var elem C.nvpair_ptr numofp := len(searchpaths) cpaths := C.alloc_cstrings(C.int(numofp)) defer C.free(unsafe.Pointer(cpaths)) @@ -271,7 +271,7 @@ func PoolImportSearch(searchpaths []string) (epools []ExportedPool, err error) { epools = make([]ExportedPool, 0, 1) for ; elem != nil; elem = C.nvlist_next_nvpair(pools, elem) { ep := ExportedPool{} - if C.nvpair_value_nvlist(elem, &config) != 0 { + if C.nvpair_value_nvlist(elem, unsafe.Pointer(&config)) != 0 { err = LastError() return } @@ -281,7 +281,7 @@ func PoolImportSearch(searchpaths []string) (epools []ExportedPool, err error) { return } ep.State = PoolState(poolState) - if C.nvlist_lookup_string(config, C.sZPOOL_CONFIG_POOL_NAME, &cname) != 0 { + if C.nvlist_lookup_string(config, C.sZPOOL_CONFIG_POOL_NAME, unsafe.Pointer(&cname)) != 0 { err = fmt.Errorf("Failed to fetch %s", C.ZPOOL_CONFIG_POOL_NAME) return } @@ -294,12 +294,12 @@ func PoolImportSearch(searchpaths []string) (epools []ExportedPool, err error) { reason = C.zpool_import_status(config, &msgid, &errata) ep.Status = PoolStatus(reason) - if C.nvlist_lookup_string(config, C.sZPOOL_CONFIG_COMMENT, &comment) == 0 { + if C.nvlist_lookup_string(config, C.sZPOOL_CONFIG_COMMENT, unsafe.Pointer(&comment)) == 0 { ep.Comment = C.GoString(comment) } if C.nvlist_lookup_nvlist(config, C.sZPOOL_CONFIG_VDEV_TREE, - &nvroot) != 0 { + unsafe.Pointer(&nvroot)) != 0 { err = fmt.Errorf("Failed to fetch %s", C.ZPOOL_CONFIG_VDEV_TREE) return } @@ -311,8 +311,8 @@ func PoolImportSearch(searchpaths []string) (epools []ExportedPool, err error) { func poolSearchImport(q string, searchpaths []string, guid bool) (name string, err error) { - var config *C.nvlist_t - var cname *C.char + var config C.nvlist_ptr + var cname C.char_ptr config = nil errPoolList := errors.New("Failed to list pools") var elem *C.nvpair_t @@ -332,7 +332,7 @@ func poolSearchImport(q string, searchpaths []string, guid bool) (name string, for ; elem != nil; elem = C.nvlist_next_nvpair(pools, elem) { var cq *C.char var tconfig *C.nvlist_t - retcode := C.nvpair_value_nvlist(elem, &tconfig) + retcode := C.nvpair_value_nvlist(elem, unsafe.Pointer(&tconfig)) if retcode != 0 { err = errPoolList return @@ -351,7 +351,7 @@ func poolSearchImport(q string, searchpaths []string, guid bool) (name string, } } else { if retcode = C.nvlist_lookup_string(tconfig, - C.sZPOOL_CONFIG_POOL_NAME, &cq); retcode != 0 { + C.sZPOOL_CONFIG_POOL_NAME, unsafe.Pointer(&cq)); retcode != 0 { err = errPoolList return } @@ -370,7 +370,7 @@ func poolSearchImport(q string, searchpaths []string, guid bool) (name string, if guid { // We need to get name so we can open pool by name if retcode := C.nvlist_lookup_string(config, - C.sZPOOL_CONFIG_POOL_NAME, &cname); retcode != 0 { + C.sZPOOL_CONFIG_POOL_NAME, unsafe.Pointer(&cname)); retcode != 0 { err = errPoolList return } @@ -416,7 +416,7 @@ func PoolImportByGUID(guid string, searchpaths []string) (pool Pool, err error) // anymore. Call Pool.Close() method. func PoolOpenAll() (pools []Pool, err error) { var pool Pool - errcode := C.zpool_list(libzfsHandle, &pool.list) + errcode := C.zpool_list(libzfsHandle, unsafe.Pointer(&pool.list)) for pool.list != nil { err = pool.ReloadProperties() if err != nil { @@ -516,7 +516,7 @@ func (pool *Pool) GetProperty(p Prop) (prop Property, err error) { return } var list C.property_list_t - r := C.read_zpool_property(pool.list.zph, &list, C.int(p)) + r := C.read_zpool_property(pool.list.zph, unsafe.Pointer(&list), C.int(p)) if r != 0 { err = LastError() } @@ -632,12 +632,12 @@ func (vdev *VDevTree) isLog() (r C.uint64_t) { return } -func toCPoolProperties(props PoolProperties) (cprops *C.nvlist_t) { +func toCPoolProperties(props PoolProperties) (cprops C.nvlist_ptr) { cprops = nil for prop, value := range props { name := C.zpool_prop_to_name(C.zpool_prop_t(prop)) csPropValue := C.CString(value) - r := C.add_prop_list(name, csPropValue, &cprops, C.boolean_t(1)) + r := C.add_prop_list(name, csPropValue, unsafe.Pointer(&cprops), C.boolean_t(1)) C.free(unsafe.Pointer(csPropValue)) if r != 0 { if cprops != nil { @@ -650,12 +650,12 @@ func toCPoolProperties(props PoolProperties) (cprops *C.nvlist_t) { return } -func toCDatasetProperties(props DatasetProperties) (cprops *C.nvlist_t) { +func toCDatasetProperties(props DatasetProperties) (cprops C.nvlist_ptr) { cprops = nil for prop, value := range props { name := C.zfs_prop_to_name(C.zfs_prop_t(prop)) csPropValue := C.CString(value) - r := C.add_prop_list(name, csPropValue, &cprops, C.boolean_t(0)) + r := C.add_prop_list(name, csPropValue, unsafe.Pointer(&cprops), C.boolean_t(0)) C.free(unsafe.Pointer(csPropValue)) if r != 0 { if cprops != nil { @@ -696,9 +696,9 @@ func buildVDevTree(root *C.nvlist_t, rtype VDevType, vdevs []VDevTree, defer C.nvlist_free_array(l2cache) for i, vdev := range vdevs { grouping, mindevs, maxdevs := vdev.isGrouping() - var child *C.nvlist_t + var child C.nvlist_ptr // fmt.Println(vdev.Type) - if r := C.nvlist_alloc(&child, C.NV_UNIQUE_NAME, 0); r != 0 { + if r := C.nvlist_alloc(unsafe.Pointer(&child), C.NV_UNIQUE_NAME, 0); r != 0 { err = errors.New("Failed to allocate vdev") return } @@ -812,8 +812,8 @@ func buildVDevTree(root *C.nvlist_t, rtype VDevType, vdevs []VDevTree, func PoolCreate(name string, vdevs []VDevTree, features map[string]string, props PoolProperties, fsprops DatasetProperties) (pool Pool, err error) { // create root vdev nvroot - var nvroot *C.nvlist_t - if r := C.nvlist_alloc(&nvroot, C.NV_UNIQUE_NAME, 0); r != 0 { + var nvroot C.nvlist_ptr + if r := C.nvlist_alloc(unsafe.Pointer(&nvroot), C.NV_UNIQUE_NAME, 0); r != 0 { err = errors.New("Failed to allocate root vdev") return } @@ -860,7 +860,7 @@ func PoolCreate(name string, vdevs []VDevTree, features map[string]string, for fname, fval := range features { csName := C.CString(fmt.Sprintf("feature@%s", fname)) csVal := C.CString(fval) - r := C.add_prop_list(csName, csVal, &cprops, + r := C.add_prop_list(csName, csVal, unsafe.Pointer(&cprops), C.boolean_t(1)) C.free(unsafe.Pointer(csName)) C.free(unsafe.Pointer(csVal)) @@ -890,14 +890,14 @@ func PoolCreate(name string, vdevs []VDevTree, features map[string]string, // Status get pool status. Let you check if pool healthy. func (pool *Pool) Status() (status PoolStatus, err error) { - var msgid *C.char + var msgid C.char_ptr var reason C.zpool_status_t var errata C.zpool_errata_t if pool.list == nil { err = errors.New(msgPoolIsNil) return } - reason = C.zpool_get_status(pool.list.zph, &msgid, &errata) + reason = C.zpool_get_status(pool.list.zph, unsafe.Pointer(&msgid), &errata) status = PoolStatus(reason) return } @@ -948,7 +948,7 @@ func (pool *Pool) ExportForce(log string) (err error) { // VDevTree - Fetch pool's current vdev tree configuration, state and stats func (pool *Pool) VDevTree() (vdevs VDevTree, err error) { - var nvroot *C.nvlist_t + var nvroot C.nvlist_ptr var poolName string config := C.zpool_get_config(pool.list.zph, nil) if config == nil { @@ -956,7 +956,7 @@ func (pool *Pool) VDevTree() (vdevs VDevTree, err error) { return } if C.nvlist_lookup_nvlist(config, C.sZPOOL_CONFIG_VDEV_TREE, - &nvroot) != 0 { + unsafe.Pointer(&nvroot)) != 0 { err = fmt.Errorf("Failed to fetch %s", C.ZPOOL_CONFIG_VDEV_TREE) return } diff --git a/zpool.h b/zpool.h index 7fbb190..3aaf3c8 100644 --- a/zpool.h +++ b/zpool.h @@ -23,6 +23,15 @@ typedef struct property_list { } property_list_t; typedef struct zpool_list zpool_list_t; +typedef struct zpool_list* zpool_list_ptr; +typedef struct libzfs_handle* libzfs_handle_ptr; +typedef struct nvlist* nvlist_ptr; +typedef struct property_list *property_list_ptr; +typedef struct pool_scan_stat* pool_scan_stat_ptr; +typedef struct nvpair* nvpair_ptr; + +typedef struct vdev_stat* vdev_stat_ptr; +typedef char* char_ptr; property_list_t *new_property_list();