From a6fba76e816fd3c7f316e6ed22cc86395acf1eee Mon Sep 17 00:00:00 2001 From: Faruk Kasumovic Date: Tue, 26 Jul 2016 21:14:28 +0200 Subject: [PATCH] - Fix remaining cgo CString memory leaks --- zfs.c | 2 +- zfs.go | 40 ++++++++++++++++++------ zfs.h | 2 +- zpool.c | 4 --- zpool.go | 94 +++++++++++++++++++++++++++++++++++++------------------- zpool.h | 3 -- 6 files changed, 94 insertions(+), 51 deletions(-) diff --git a/zfs.c b/zfs.c index 46b7721..047ba48 100644 --- a/zfs.c +++ b/zfs.c @@ -87,7 +87,7 @@ int clear_last_error(libzfs_handle_t *hdl) { return 0; } -char** alloc_strings(int size) { +char** alloc_cstrings(int size) { return malloc(size*sizeof(char*)); } diff --git a/zfs.go b/zfs.go index 288fab7..82dc968 100644 --- a/zfs.go +++ b/zfs.go @@ -8,6 +8,7 @@ import "C" import ( "errors" + "unsafe" ) const ( @@ -104,7 +105,9 @@ func DatasetCloseAll(datasets []Dataset) { // DatasetOpen open dataset and all of its recursive children datasets func DatasetOpen(path string) (d Dataset, err error) { d.list = C.create_dataset_list_item() - d.list.zh = C.zfs_open(libzfsHandle, C.CString(path), 0xF) + csPath := C.CString(path) + d.list.zh = C.zfs_open(libzfsHandle, csPath, 0xF) + C.free(unsafe.Pointer(csPath)) if d.list.zh == nil { err = LastError() @@ -129,9 +132,11 @@ func datasetPropertiesTonvlist(props map[Prop]Property) ( return } for prop, value := range props { + csValue := C.CString(value.Value) r := C.nvlist_add_string( cprops, C.zfs_prop_to_name( - C.zfs_prop_t(prop)), C.CString(value.Value)) + C.zfs_prop_t(prop)), csValue) + C.free(unsafe.Pointer(csValue)) if r != 0 { err = errors.New("Failed to convert property") return @@ -150,8 +155,10 @@ func DatasetCreate(path string, dtype DatasetType, } defer C.nvlist_free(cprops) - errcode := C.zfs_create(libzfsHandle, C.CString(path), + csPath := C.CString(path) + errcode := C.zfs_create(libzfsHandle, csPath, C.zfs_type_t(dtype), cprops) + C.free(unsafe.Pointer(csPath)) if errcode != 0 { err = LastError() } @@ -179,6 +186,9 @@ func (d *Dataset) Destroy(Defer bool) (err error) { return } dsType, e := d.GetProperty(DatasetPropType) + if e != nil { + dsType.Value = err.Error() // just put error (why it didn't fetch property type) + } err = errors.New("Cannot destroy dataset " + path + ": " + dsType.Value + " has children") return @@ -276,8 +286,10 @@ func (d *Dataset) SetProperty(p Prop, value string) (err error) { err = errors.New(msgDatasetIsNil) return } + csValue := C.CString(value) errcode := C.zfs_prop_set(d.list.zh, C.zfs_prop_to_name( - C.zfs_prop_t(p)), C.CString(value)) + C.zfs_prop_t(p)), csValue) + C.free(unsafe.Pointer(csValue)) if errcode != 0 { err = LastError() } @@ -300,7 +312,9 @@ func (d *Dataset) Clone(target string, props map[Prop]Property) (rd Dataset, err return } defer C.nvlist_free(cprops) - if errc := C.zfs_clone(d.list.zh, C.CString(target), cprops); errc != 0 { + csTarget := C.CString(target) + defer C.free(unsafe.Pointer(csTarget)) + if errc := C.zfs_clone(d.list.zh, csTarget, cprops); errc != 0 { err = LastError() return } @@ -315,7 +329,9 @@ func DatasetSnapshot(path string, recur bool, props map[Prop]Property) (rd Datas return } defer C.nvlist_free(cprops) - if errc := C.zfs_snapshot(libzfsHandle, C.CString(path), booleanT(recur), cprops); errc != 0 { + csPath := C.CString(path) + defer C.free(unsafe.Pointer(csPath)) + if errc := C.zfs_snapshot(libzfsHandle, csPath, booleanT(recur), cprops); errc != 0 { err = LastError() return } @@ -348,13 +364,15 @@ func (d *Dataset) Rollback(snap *Dataset, force bool) (err error) { } // Rename dataset -func (d *Dataset) Rename(newname string, recur, +func (d *Dataset) Rename(newName string, recur, forceUnmount bool) (err error) { if d.list == nil { err = errors.New(msgDatasetIsNil) return } - if errc := C.zfs_rename(d.list.zh, C.CString(newname), + csNewName := C.CString(newName) + defer C.free(unsafe.Pointer(csNewName)) + if errc := C.zfs_rename(d.list.zh, csNewName, booleanT(recur), booleanT(forceUnmount)); errc != 0 { err = LastError() } @@ -370,7 +388,7 @@ func (d *Dataset) IsMounted() (mounted bool, where string) { return false, "" } m := C.zfs_is_mounted(d.list.zh, &cw) - defer C.free_cstring(cw) + defer C.free(unsafe.Pointer(cw)) if m != 0 { return true, C.GoString(cw) } @@ -383,7 +401,9 @@ func (d *Dataset) Mount(options string, flags int) (err error) { err = errors.New(msgDatasetIsNil) return } - if ec := C.zfs_mount(d.list.zh, C.CString(options), C.int(flags)); ec != 0 { + csOptions := C.CString(options) + defer C.free(unsafe.Pointer(csOptions)) + if ec := C.zfs_mount(d.list.zh, csOptions, C.int(flags)); ec != 0 { err = LastError() } return diff --git a/zfs.h b/zfs.h index df9403a..40a4cd5 100644 --- a/zfs.h +++ b/zfs.h @@ -24,7 +24,7 @@ int read_dataset_property(zfs_handle_t *zh, property_list_t *list, int prop); int clear_last_error(libzfs_handle_t *libzfs); -char** alloc_strings(int size); +char** alloc_cstrings(int size); void strings_setat(char **a, int at, char *v); #endif diff --git a/zpool.c b/zpool.c index 08a908e..5755312 100644 --- a/zpool.c +++ b/zpool.c @@ -462,10 +462,6 @@ void nvlist_free_array(nvlist_t **a) { free(a); } -void free_cstring(char *str) { - free(str); -} - nvlist_t *nvlist_array_at(nvlist_t **a, uint_t i) { return a[i]; } diff --git a/zpool.go b/zpool.go index a8783bd..55e221c 100644 --- a/zpool.go +++ b/zpool.go @@ -11,6 +11,7 @@ import ( "fmt" "strconv" "time" + "unsafe" ) const ( @@ -129,9 +130,9 @@ type Pool struct { // Returns Pool object, requires Pool.Close() to be called explicitly // for memory cleanup after object is not needed anymore. func PoolOpen(name string) (pool Pool, err error) { - namestr := C.CString(name) - pool.list = C.zpool_list_open(libzfsHandle, namestr) - C.free_cstring(namestr) + csName := C.CString(name) + defer C.free(unsafe.Pointer(csName)) + pool.list = C.zpool_list_open(libzfsHandle, csName) if pool.list != nil { err = pool.ReloadProperties() @@ -227,11 +228,10 @@ func poolGetConfig(name string, nv *C.nvlist_t) (vdevs VDevTree, err error) { } vname := C.zpool_vdev_name(libzfsHandle, nil, C.nvlist_array_at(child, c), C.B_TRUE) - var vdev VDevTree vdev, err = poolGetConfig(C.GoString(vname), C.nvlist_array_at(child, c)) - C.free_cstring(vname) + C.free(unsafe.Pointer(vname)) if err != nil { return } @@ -251,9 +251,12 @@ func PoolImportSearch(searchpaths []string) (epools []ExportedPool, err error) { config = nil var elem *C.nvpair_t numofp := len(searchpaths) - cpaths := C.alloc_strings(C.int(numofp)) + cpaths := C.alloc_cstrings(C.int(numofp)) + defer C.free(unsafe.Pointer(cpaths)) for i, path := range searchpaths { - C.strings_setat(cpaths, C.int(i), C.CString(path)) + csPath := C.CString(path) + defer C.free(unsafe.Pointer(csPath)) + C.strings_setat(cpaths, C.int(i), csPath) } pools := C.zpool_find_import(libzfsHandle, C.int(numofp), cpaths) @@ -308,9 +311,12 @@ func poolSearchImport(q string, searchpaths []string, guid bool) (name string, errPoolList := errors.New("Failed to list pools") var elem *C.nvpair_t numofp := len(searchpaths) - cpaths := C.alloc_strings(C.int(numofp)) + cpaths := C.alloc_cstrings(C.int(numofp)) + defer C.free(unsafe.Pointer(cpaths)) for i, path := range searchpaths { - C.strings_setat(cpaths, C.int(i), C.CString(path)) + csPath := C.CString(path) + defer C.free(unsafe.Pointer(csPath)) + C.strings_setat(cpaths, C.int(i), csPath) } pools := C.zpool_find_import(libzfsHandle, C.int(numofp), cpaths) @@ -446,7 +452,7 @@ func PoolStateToName(state PoolState) (name string) { return } -// Refresh the pool's vdev statistics, e.g. bytes read/written. +// RefreshStats the pool's vdev statistics, e.g. bytes read/written. func (pool *Pool) RefreshStats() (err error) { if 0 != C.refresh_stats(pool.list) { return errors.New("error refreshing stats") @@ -520,11 +526,9 @@ func (pool *Pool) GetProperty(p Prop) (prop Property, err error) { // feature in Features map. func (pool *Pool) GetFeature(name string) (value string, err error) { var fvalue [512]C.char - var sname *C.char - sname = C.CString(fmt.Sprint("feature@", name)) - r := C.zpool_prop_get_feature(pool.list.zph, sname, &(fvalue[0]), 512) - C.free_cstring(sname) - + csName := C.CString(fmt.Sprint("feature@", name)) + r := C.zpool_prop_get_feature(pool.list.zph, csName, &(fvalue[0]), 512) + C.free(unsafe.Pointer(csName)) if r != 0 { err = errors.New(fmt.Sprint("Unknown zpool feature: ", name)) return @@ -545,7 +549,11 @@ func (pool *Pool) SetProperty(p Prop, value string) (err error) { PoolPropertyToName(p))) return } - r := C.zpool_set_prop(pool.list.zph, C.CString(PoolPropertyToName(p)), C.CString(value)) + csPropName := C.CString(PoolPropertyToName(p)) + csPropValue := C.CString(value) + r := C.zpool_set_prop(pool.list.zph, csPropName, csPropValue) + C.free(unsafe.Pointer(csPropName)) + C.free(unsafe.Pointer(csPropValue)) if r != 0 { err = LastError() } else { @@ -622,7 +630,9 @@ func toCPoolProperties(props PoolProperties) (cprops *C.nvlist_t) { cprops = nil for prop, value := range props { name := C.zpool_prop_to_name(C.zpool_prop_t(prop)) - r := C.add_prop_list(name, C.CString(value), &cprops, C.boolean_t(1)) + csPropValue := C.CString(value) + r := C.add_prop_list(name, csPropValue, &cprops, C.boolean_t(1)) + C.free(unsafe.Pointer(csPropValue)) if r != 0 { if cprops != nil { C.nvlist_free(cprops) @@ -638,7 +648,9 @@ func toCDatasetProperties(props DatasetProperties) (cprops *C.nvlist_t) { cprops = nil for prop, value := range props { name := C.zfs_prop_to_name(C.zfs_prop_t(prop)) - r := C.add_prop_list(name, C.CString(value), &cprops, C.boolean_t(0)) + csPropValue := C.CString(value) + r := C.add_prop_list(name, csPropValue, &cprops, C.boolean_t(0)) + C.free(unsafe.Pointer(csPropValue)) if r != 0 { if cprops != nil { C.nvlist_free(cprops) @@ -691,8 +703,11 @@ func buildVDevTree(root *C.nvlist_t, rtype VDevType, vdevs []VDevTree, vdev.Type, mindevs, maxdevs) return } - if r := C.nvlist_add_string(child, C.sZPOOL_CONFIG_TYPE, - C.CString(string(vdev.Type))); r != 0 { + csType := C.CString(string(vdev.Type)) + r := C.nvlist_add_string(child, C.sZPOOL_CONFIG_TYPE, + csType) + C.free(unsafe.Pointer(csType)) + if r != 0 { err = errors.New("Failed to set vdev type") return } @@ -724,9 +739,12 @@ func buildVDevTree(root *C.nvlist_t, rtype VDevType, vdevs []VDevTree, } // } if len(vdev.Path) > 0 { - if r := C.nvlist_add_string( + csPath := C.CString(vdev.Path) + r := C.nvlist_add_string( child, C.sZPOOL_CONFIG_PATH, - C.CString(vdev.Path)); r != 0 { + csPath) + C.free(unsafe.Pointer(csPath)) + if r != 0 { err = errors.New("Failed to allocate vdev child (type)") return } @@ -793,8 +811,11 @@ func PoolCreate(name string, vdevs []VDevTree, features map[string]string, err = errors.New("Failed to allocate root vdev") return } - if r := C.nvlist_add_string(nvroot, C.sZPOOL_CONFIG_TYPE, - C.CString(string(VDevTypeRoot))); r != 0 { + csTypeRoot := C.CString(string(VDevTypeRoot)) + r := C.nvlist_add_string(nvroot, C.sZPOOL_CONFIG_TYPE, + csTypeRoot) + C.free(unsafe.Pointer(csTypeRoot)) + if r != 0 { err = errors.New("Failed to allocate root vdev") return } @@ -821,9 +842,12 @@ func PoolCreate(name string, vdevs []VDevTree, features map[string]string, return } for fname, fval := range features { - sfname := fmt.Sprintf("feature@%s", fname) - r := C.add_prop_list(C.CString(sfname), C.CString(fval), &cprops, + csName := C.CString(fmt.Sprintf("feature@%s", fname)) + csVal := C.CString(fval) + r := C.add_prop_list(csName, csVal, &cprops, C.boolean_t(1)) + C.free(unsafe.Pointer(csName)) + C.free(unsafe.Pointer(csVal)) if r != 0 { if cprops != nil { C.nvlist_free(cprops) @@ -834,7 +858,9 @@ func PoolCreate(name string, vdevs []VDevTree, features map[string]string, } // Create actual pool then open - if r := C.zpool_create(libzfsHandle, C.CString(name), nvroot, + csName := C.CString(name) + defer C.free(unsafe.Pointer(csName)) + if r := C.zpool_create(libzfsHandle, csName, nvroot, cprops, cfsprops); r != 0 { err = LastError() err = errors.New(err.Error() + " (zpool_create)") @@ -868,7 +894,9 @@ func (pool *Pool) Destroy(logStr string) (err error) { err = errors.New(msgPoolIsNil) return } - retcode := C.zpool_destroy(pool.list.zph, C.CString(logStr)) + csLog := C.CString(logStr) + defer C.free(unsafe.Pointer(csLog)) + retcode := C.zpool_destroy(pool.list.zph, csLog) if retcode != 0 { err = LastError() } @@ -884,7 +912,9 @@ func (pool *Pool) Export(force bool, log string) (err error) { if force { forcet = 1 } - if rc := C.zpool_export(pool.list.zph, forcet, C.CString(log)); rc != 0 { + csLog := C.CString(log) + defer C.free(unsafe.Pointer(csLog)) + if rc := C.zpool_export(pool.list.zph, forcet, csLog); rc != 0 { err = LastError() } return @@ -892,11 +922,11 @@ func (pool *Pool) Export(force bool, log string) (err error) { // ExportForce hard force export of the pool from the system. func (pool *Pool) ExportForce(log string) (err error) { - logstr := C.CString(log) - if rc := C.zpool_export_force(pool.list.zph, logstr); rc != 0 { + csLog := C.CString(log) + defer C.free(unsafe.Pointer(csLog)) + if rc := C.zpool_export_force(pool.list.zph, csLog); rc != 0 { err = LastError() } - C.free_cstring(logstr) return } diff --git a/zpool.h b/zpool.h index b36ce3d..c45e5b7 100644 --- a/zpool.h +++ b/zpool.h @@ -52,9 +52,6 @@ void nvlist_array_set(nvlist_t** a, int i, nvlist_t *item); void nvlist_free_array(nvlist_t **a); nvlist_t *nvlist_array_at(nvlist_t **a, uint_t i); - -void free_cstring(char *str); - int nvlist_lookup_uint64_array_vds(nvlist_t *nv, const char *p, vdev_stat_t **vds, uint_t *c);