From b86c79329b7bf759c9781fea44327d9af359f250 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Fri, 29 Oct 2021 15:49:30 +0300 Subject: [PATCH] os, builtin: reduce leaks without -autofree --- vlib/builtin/array.v | 17 +++++++--- vlib/builtin/string.v | 16 +++++++++- vlib/os/os.c.v | 73 ++++++++++++++++++++++++++++++++++++------- vlib/os/os.v | 11 +++++-- vlib/os/os_nix.c.v | 6 +++- 5 files changed, 103 insertions(+), 20 deletions(-) diff --git a/vlib/builtin/array.v b/vlib/builtin/array.v index 5b30541018..a0a2561505 100644 --- a/vlib/builtin/array.v +++ b/vlib/builtin/array.v @@ -544,18 +544,25 @@ pub fn (mut a []string) free() { // => '["a", "b", "c"]'. [manualfree] pub fn (a []string) str() string { - mut sb := strings.new_builder(a.len * 3) - sb.write_string('[') + mut sb_len := 4 // 2x" + 1x, + 1xspace + if a.len > 0 { + // assume that most strings will be ~large as the first + sb_len += a[0].len + sb_len *= a.len + } + sb_len += 2 // 1x[ + 1x] + mut sb := strings.new_builder(sb_len) + sb.write_b(`[`) for i in 0 .. a.len { val := a[i] - sb.write_string("'") + sb.write_b(`'`) sb.write_string(val) - sb.write_string("'") + sb.write_b(`'`) if i < a.len - 1 { sb.write_string(', ') } } - sb.write_string(']') + sb.write_b(`]`) res := sb.str() unsafe { sb.free() } return res diff --git a/vlib/builtin/string.v b/vlib/builtin/string.v index 2182ecf553..50659192c8 100644 --- a/vlib/builtin/string.v +++ b/vlib/builtin/string.v @@ -55,6 +55,9 @@ mut: is_lit int } +// runes returns an array of all the utf runes in the string `s` +// which is useful if you want random access to them +[direct_array_access] pub fn (s string) runes() []rune { mut runes := []rune{cap: s.len} for i := 0; i < s.len; i++ { @@ -350,7 +353,7 @@ pub fn (s string) replace_each(vals []string) string { // Remember positions of all rep strings, and calculate the length // of the new string to do just one allocation. mut new_len := s.len - mut idxs := []RepIndex{} + mut idxs := []RepIndex{cap: 6} mut idx := 0 s_ := s.clone() for rep_i := 0; rep_i < vals.len; rep_i += 2 { @@ -672,6 +675,7 @@ fn (s string) substr2(start int, _end int, end_max bool) string { // substr returns the string between index positions `start` and `end`. // Example: assert 'ABCD'.substr(1,3) == 'BC' +[direct_array_access] pub fn (s string) substr(start int, end int) string { $if !no_bounds_checking ? { if start > end || start > s.len || end > s.len || start < 0 || end < 0 { @@ -699,6 +703,7 @@ pub fn (s string) substr(start int, end int) string { // index returns the position of the first character of the input string. // It will return `-1` if the input string can't be found. +[direct_array_access] fn (s string) index_(p string) int { if p.len > s.len || p.len == 0 { return -1 @@ -778,6 +783,7 @@ pub fn (s string) index_any(chars string) int { } // last_index returns the position of the last occurence of the input string. +[direct_array_access] fn (s string) last_index_(p string) int { if p.len > s.len || p.len == 0 { return -1 @@ -806,6 +812,7 @@ pub fn (s string) last_index(p string) ?int { } // index_after returns the position of the input string, starting search from `start` position. +[direct_array_access] pub fn (s string) index_after(p string, start int) int { if p.len > s.len { return -1 @@ -835,6 +842,7 @@ pub fn (s string) index_after(p string, start int) int { // index_byte returns the index of byte `c` if found in the string. // index_byte returns -1 if the byte can not be found. +[direct_array_access] pub fn (s string) index_byte(c byte) int { for i in 0 .. s.len { if unsafe { s.str[i] } == c { @@ -846,6 +854,7 @@ pub fn (s string) index_byte(c byte) int { // last_index_byte returns the index of the last occurence of byte `c` if found in the string. // last_index_byte returns -1 if the byte is not found. +[direct_array_access] pub fn (s string) last_index_byte(c byte) int { for i := s.len - 1; i >= 0; i-- { if unsafe { s.str[i] == c } { @@ -857,6 +866,7 @@ pub fn (s string) last_index_byte(c byte) int { // count returns the number of occurrences of `substr` in the string. // count returns -1 if no `substr` could be found. +[direct_array_access] pub fn (s string) count(substr string) int { if s.len == 0 || substr.len == 0 { return 0 @@ -926,6 +936,7 @@ pub fn (s string) contains_any_substr(substrs []string) bool { } // starts_with returns `true` if the string starts with `p`. +[direct_array_access] pub fn (s string) starts_with(p string) bool { if p.len > s.len { return false @@ -939,6 +950,7 @@ pub fn (s string) starts_with(p string) bool { } // ends_with returns `true` if the string ends with `p`. +[direct_array_access] pub fn (s string) ends_with(p string) bool { if p.len > s.len { return false @@ -953,6 +965,7 @@ pub fn (s string) ends_with(p string) bool { // to_lower returns the string in all lowercase characters. // TODO only works with ASCII +[direct_array_access] pub fn (s string) to_lower() string { unsafe { mut b := malloc_noscan(s.len + 1) @@ -982,6 +995,7 @@ pub fn (s string) is_lower() bool { // to_upper returns the string in all uppercase characters. // Example: assert 'Hello V'.to_upper() == 'HELLO V' +[direct_array_access] pub fn (s string) to_upper() string { unsafe { mut b := malloc_noscan(s.len + 1) diff --git a/vlib/os/os.c.v b/vlib/os/os.c.v index 79c886c1dd..fd9f6e0b52 100644 --- a/vlib/os/os.c.v +++ b/vlib/os/os.c.v @@ -419,20 +419,30 @@ pub fn is_executable(path string) bool { } // is_writable returns `true` if `path` is writable. +[manualfree] pub fn is_writable(path string) bool { $if windows { p := path.replace('/', '\\') - return C._waccess(p.to_wide(), w_ok) != -1 + wp := p.to_wide() + res := C._waccess(wp, w_ok) != -1 + unsafe { wp.free() } + unsafe { p.free() } + return res } $else { return C.access(&char(path.str), w_ok) != -1 } } // is_readable returns `true` if `path` is readable. +[manualfree] pub fn is_readable(path string) bool { $if windows { p := path.replace('/', '\\') - return C._waccess(p.to_wide(), r_ok) != -1 + wp := p.to_wide() + res := C._waccess(wp, r_ok) != -1 + unsafe { wp.free() } + unsafe { p.free() } + return res } $else { return C.access(&char(path.str), r_ok) != -1 } @@ -612,17 +622,24 @@ pub fn on_segfault(f voidptr) { pub fn executable() string { $if linux { mut xresult := vcalloc_noscan(max_path_len) + defer { + unsafe { free(xresult) } + } count := C.readlink(c'/proc/self/exe', &char(xresult), max_path_len) if count < 0 { eprintln('os.executable() failed at reading /proc/self/exe to get exe path') return executable_fallback() } - return unsafe { xresult.vstring() } + res := unsafe { cstring_to_vstring(xresult) } + return res } $if windows { max := 512 size := max * 2 // max_path_len * sizeof(wchar_t) mut result := unsafe { &u16(vcalloc_noscan(size)) } + defer { + unsafe { free(result) } + } len := C.GetModuleFileName(0, result, max) // determine if the file is a windows symlink attrs := C.GetFileAttributesW(result) @@ -648,20 +665,28 @@ pub fn executable() string { } $if macos { mut result := vcalloc_noscan(max_path_len) + defer { + unsafe { free(result) } + } pid := C.getpid() ret := proc_pidpath(pid, result, max_path_len) if ret <= 0 { eprintln('os.executable() failed at calling proc_pidpath with pid: $pid . proc_pidpath returned $ret ') return executable_fallback() } - return unsafe { result.vstring() } + res := unsafe { cstring_to_vstring(result) } + return res } $if freebsd { mut result := vcalloc_noscan(max_path_len) + defer { + unsafe { free(result) } + } mib := [1 /* CTL_KERN */, 14 /* KERN_PROC */, 12 /* KERN_PROC_PATHNAME */, -1] size := max_path_len unsafe { C.sysctl(mib.data, 4, result, &size, 0, 0) } - return unsafe { result.vstring() } + res := unsafe { cstring_to_vstring(result) } + return res } // "Sadly there is no way to get the full path of the executed file in OpenBSD." $if openbsd { @@ -672,21 +697,29 @@ pub fn executable() string { } $if netbsd { mut result := vcalloc_noscan(max_path_len) + defer { + unsafe { free(result) } + } count := C.readlink(c'/proc/curproc/exe', &char(result), max_path_len) if count < 0 { eprintln('os.executable() failed at reading /proc/curproc/exe to get exe path') return executable_fallback() } - return unsafe { result.vstring_with_len(count) } + res := unsafe { cstring_to_vstring(result) } + return res } $if dragonfly { mut result := vcalloc_noscan(max_path_len) + defer { + unsafe { free(result) } + } count := C.readlink(c'/proc/curproc/file', &char(result), max_path_len) if count < 0 { eprintln('os.executable() failed at reading /proc/curproc/file to get exe path') return executable_fallback() } - return unsafe { result.vstring_with_len(count) } + res := unsafe { cstring_to_vstring(result) } + return res } return executable_fallback() } @@ -738,6 +771,7 @@ pub fn chdir(path string) ? { } // getwd returns the absolute path of the current directory. +[manualfree] pub fn getwd() string { $if windows { max := 512 // max_path_len * sizeof(wchar_t) @@ -747,7 +781,9 @@ pub fn getwd() string { free(buf) return '' } - return string_from_wide(buf) + res := string_from_wide(buf) + free(buf) + return res } } $else { buf := vcalloc_noscan(max_path_len) @@ -756,7 +792,9 @@ pub fn getwd() string { free(buf) return '' } - return buf.vstring() + res := cstring_to_vstring(buf) + free(buf) + return res } } } @@ -782,10 +820,13 @@ pub fn real_path(fpath string) string { C.CloseHandle(file) if final_len < size { rt := unsafe { string_from_wide2(fullpath, final_len) } + unsafe { free(fullpath) } + unsafe { res.free() } res = rt[4..] } else { - unsafe { free(fullpath) } eprintln('os.real_path() saw that the file path was too long') + unsafe { res.free() } + unsafe { free(fullpath) } return fpath.clone() } } else { @@ -794,19 +835,29 @@ pub fn real_path(fpath string) string { // TODO: check errors if path len is not enough ret := C.GetFullPathName(fpath.to_wide(), max_path_len, fullpath, 0) if ret == 0 { + unsafe { res.free() } unsafe { free(fullpath) } return fpath.clone() } + unsafe { res.free() } res = unsafe { string_from_wide(fullpath) } + unsafe { free(fullpath) } } } $else { mut fullpath := vcalloc_noscan(max_path_len) ret := &char(C.realpath(&char(fpath.str), &char(fullpath))) if ret == 0 { + unsafe { res.free() } unsafe { free(fullpath) } return fpath.clone() } - res = unsafe { fullpath.vstring() } + // NB: fullpath is much larger (usually ~4KB), than what C.realpath will + // actually fill in the vast majority of the cases => it pays to copy the + // resulting string from that buffer, to a shorter one, and then free the + // 4KB fullpath buffer. + unsafe { res.free() } + res = unsafe { cstring_to_vstring(fullpath) } + unsafe { free(fullpath) } } unsafe { normalize_drive_letter(res) } return res diff --git a/vlib/os/os.v b/vlib/os/os.v index a7d09625a9..77b26e8544 100644 --- a/vlib/os/os.v +++ b/vlib/os/os.v @@ -148,9 +148,12 @@ pub fn rmdir_all(path string) ? { } // is_dir_empty will return a `bool` whether or not `path` is empty. +[manualfree] pub fn is_dir_empty(path string) bool { items := ls(path) or { return true } - return items.len == 0 + res := items.len == 0 + unsafe { items.free() } + return res } // file_ext will return the part after the last occurence of `.` in `path`. @@ -437,7 +440,7 @@ pub fn is_abs_path(path string) bool { // join_path returns a path as string from input string parameter(s). [manualfree] pub fn join_path(base string, dirs ...string) string { - mut result := []string{} + mut result := []string{cap: 256} result << base.trim_right('\\/') for d in dirs { result << d @@ -639,13 +642,17 @@ pub fn resource_abs_path(path string) string { mut base_path := real_path(dexe) vresource := getenv('V_RESOURCE_PATH') if vresource.len != 0 { + unsafe { base_path.free() } base_path = vresource } fp := join_path(base_path, path) res := real_path(fp) unsafe { fp.free() + vresource.free() base_path.free() + dexe.free() + exe.free() } return res } diff --git a/vlib/os/os_nix.c.v b/vlib/os/os_nix.c.v index 5c4dea0134..276d2c3a73 100644 --- a/vlib/os/os_nix.c.v +++ b/vlib/os/os_nix.c.v @@ -257,7 +257,7 @@ pub fn ls(path string) ?[]string { if path.len == 0 { return error('ls() expects a folder, not an empty string') } - mut res := []string{} + mut res := []string{cap: 50} dir := unsafe { C.opendir(&char(path.str)) } if isnil(dir) { return error('ls() couldnt open dir "$path"') @@ -472,6 +472,7 @@ pub fn debugger_present() bool { fn C.mkstemp(stemplate &byte) int // `is_writable_folder` - `folder` exists and is writable to the process +[manualfree] pub fn is_writable_folder(folder string) ?bool { if !exists(folder) { return error('`$folder` does not exist') @@ -480,6 +481,9 @@ pub fn is_writable_folder(folder string) ?bool { return error('`folder` is not a folder') } tmp_perm_check := join_path(folder, 'XXXXXX') + defer { + unsafe { tmp_perm_check.free() } + } unsafe { x := C.mkstemp(&char(tmp_perm_check.str)) if -1 == x {