vieter-v
/
v
2
0
Fork 0
Code Issues Pull Requests Releases Activity

os: re-add the leak in os.join_path (the `os.join_path(x, ...arr)` case should be handled by V). Add a memleak free os.join_path_single version.

pull/12550/head
Delyan Angelov 2021-11-22 14:40:55 +02:00
parent 5e1782bf9c
commit 7a0dc60d04
No known key found for this signature in database
GPG Key ID: 66886C0F12D595ED
7 changed files with 60 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
vlib/os/file_test.v
View File

@ -41,8 +41,8 @@ const (
) )
const ( const (
tfolder = os.join_path(os.temp_dir(), 'os_file_test') tfolder = os.join_path_single(os.temp_dir(), 'os_file_test')
tfile = os.join_path(tfolder, 'test_file') tfile = os.join_path_single(tfolder, 'test_file')
) )
fn testsuite_begin() ? { fn testsuite_begin() ? {

2
vlib/os/os.c.v
View File

@ -191,7 +191,7 @@ pub fn file_size(path string) u64 {
pub fn mv(src string, dst string) ? { pub fn mv(src string, dst string) ? {
mut rdst := dst mut rdst := dst
if is_dir(rdst) { if is_dir(rdst) {
rdst = join_path(rdst.trim_right(path_separator), file_name(src.trim_right(path_separator))) rdst = join_path_single(rdst.trim_right(path_separator), file_name(src.trim_right(path_separator)))
} }
$if windows { $if windows {
w_src := src.replace('/', '\\') w_src := src.replace('/', '\\')

44
vlib/os/os.v
View File

@ -40,7 +40,8 @@ pub fn cp_all(src string, dst string, overwrite bool) ? {
// single file copy // single file copy
if !is_dir(source_path) { if !is_dir(source_path) {
adjusted_path := if is_dir(dest_path) { adjusted_path := if is_dir(dest_path) {
join_path(dest_path, file_name(source_path)) fname := file_name(source_path)
join_path_single(dest_path, fname)
} else { } else {
dest_path dest_path
} }
@ -62,8 +63,8 @@ pub fn cp_all(src string, dst string, overwrite bool) ? {
} }
files := ls(source_path) ? files := ls(source_path) ?
for file in files { for file in files {
sp := join_path(source_path, file) sp := join_path_single(source_path, file)
dp := join_path(dest_path, file) dp := join_path_single(dest_path, file)
if is_dir(sp) { if is_dir(sp) {
if !exists(dp) { if !exists(dp) {
mkdir(dp) ? mkdir(dp) ?
@ -136,7 +137,7 @@ pub fn rmdir_all(path string) ? {
mut ret_err := '' mut ret_err := ''
items := ls(path) ? items := ls(path) ?
for item in items { for item in items {
fullpath := join_path(path, item) fullpath := join_path_single(path, item)
if is_dir(fullpath) && !is_link(fullpath) { if is_dir(fullpath) && !is_link(fullpath) {
rmdir_all(fullpath) or { ret_err = err.msg } rmdir_all(fullpath) or { ret_err = err.msg }
} else { } else {
@ -379,7 +380,7 @@ fn executable_fallback() string {
if !is_abs_path(exepath) { if !is_abs_path(exepath) {
rexepath := exepath.replace_each(['/', path_separator, r'\', path_separator]) rexepath := exepath.replace_each(['/', path_separator, r'\', path_separator])
if rexepath.contains(path_separator) { if rexepath.contains(path_separator) {
exepath = join_path(os.wd_at_startup, exepath) exepath = join_path_single(os.wd_at_startup, exepath)
} else { } else {
// no choice but to try to walk the PATH folders :-| ... // no choice but to try to walk the PATH folders :-| ...
foundpath := find_abs_path_of_executable(exepath) or { '' } foundpath := find_abs_path_of_executable(exepath) or { '' }
@ -414,7 +415,7 @@ pub fn find_abs_path_of_executable(exepath string) ?string {
path := getenv('PATH') path := getenv('PATH')
paths := path.split(path_delimiter) paths := path.split(path_delimiter)
for p in paths { for p in paths {
found_abs_path := join_path(p, exepath) found_abs_path := join_path_single(p, exepath)
if exists(found_abs_path) && is_executable(found_abs_path) { if exists(found_abs_path) && is_executable(found_abs_path) {
res = found_abs_path res = found_abs_path
break break
@ -452,9 +453,8 @@ pub fn is_abs_path(path string) bool {
// join_path returns a path as string from input string parameter(s). // join_path returns a path as string from input string parameter(s).
[manualfree] [manualfree]
pub fn join_path(base string, dirs ...string) string { pub fn join_path(base string, dirs ...string) string {
defer { // TODO: fix freeing of `dirs` when the passed arguments are variadic,
unsafe { dirs.free() } // but do not free the arr, when `os.join_path(base, ...arr)` is called.
}
mut sb := strings.new_builder(base.len + dirs.len * 50) mut sb := strings.new_builder(base.len + dirs.len * 50)
defer { defer {
unsafe { sb.free() } unsafe { sb.free() }
@ -471,6 +471,26 @@ pub fn join_path(base string, dirs ...string) string {
return sb.str() return sb.str()
} }
// join_path_single appends the `elem` after `base`, using a platform specific
// path_separator.
[manualfree]
pub fn join_path_single(base string, elem string) string {
// TODO: deprecate this and make it `return os.join_path(base, elem)`,
// when freeing variadic args vs ...arr is solved in the compiler
mut sb := strings.new_builder(base.len + elem.len + 1)
defer {
unsafe { sb.free() }
}
sbase := base.trim_right('\\/')
defer {
unsafe { sbase.free() }
}
sb.write_string(sbase)
sb.write_string(path_separator)
sb.write_string(elem)
return sb.str()
}
// walk_ext returns a recursive list of all files in `path` ending with `ext`. // walk_ext returns a recursive list of all files in `path` ending with `ext`.
pub fn walk_ext(path string, ext string) []string { pub fn walk_ext(path string, ext string) []string {
mut res := []string{} mut res := []string{}
@ -587,7 +607,7 @@ pub fn cache_dir() string {
return xdg_cache_home return xdg_cache_home
} }
} }
cdir := join_path(home_dir(), '.cache') cdir := join_path_single(home_dir(), '.cache')
if !is_dir(cdir) && !is_link(cdir) { if !is_dir(cdir) && !is_link(cdir) {
mkdir(cdir) or { panic(err) } mkdir(cdir) or { panic(err) }
} }
@ -629,7 +649,7 @@ pub fn temp_dir() string {
fn default_vmodules_path() string { fn default_vmodules_path() string {
hdir := home_dir() hdir := home_dir()
res := join_path(hdir, '.vmodules') res := join_path_single(hdir, '.vmodules')
return res return res
} }
@ -684,7 +704,7 @@ pub fn resource_abs_path(path string) string {
unsafe { base_path.free() } unsafe { base_path.free() }
base_path = vresource base_path = vresource
} }
fp := join_path(base_path, path) fp := join_path_single(base_path, path)
res := real_path(fp) res := real_path(fp)
unsafe { unsafe {
fp.free() fp.free()

2
vlib/os/os_nix.c.v
View File

@ -480,7 +480,7 @@ pub fn is_writable_folder(folder string) ?bool {
if !is_dir(folder) { if !is_dir(folder) {
return error('`folder` is not a folder') return error('`folder` is not a folder')
} }
tmp_perm_check := join_path(folder, 'XXXXXX') tmp_perm_check := join_path_single(folder, 'XXXXXX')
defer { defer {
unsafe { tmp_perm_check.free() } unsafe { tmp_perm_check.free() }
} }

38
vlib/os/os_test.v
View File

@ -106,9 +106,9 @@ fn test_create_file() ? {
fn test_is_file() { fn test_is_file() {
// Setup // Setup
work_dir := os.join_path(os.getwd(), 'is_file_test') work_dir := os.join_path_single(os.getwd(), 'is_file_test')
os.mkdir_all(work_dir) or { panic(err) } os.mkdir_all(work_dir) or { panic(err) }
tfile := os.join_path(work_dir, 'tmp_file') tfile := os.join_path_single(work_dir, 'tmp_file')
// Test things that shouldn't be a file // Test things that shouldn't be a file
assert os.is_file(work_dir) == false assert os.is_file(work_dir) == false
assert os.is_file('non-existent_file.tmp') == false assert os.is_file('non-existent_file.tmp') == false
@ -120,7 +120,7 @@ fn test_is_file() {
$if windows { $if windows {
assert true assert true
} $else { } $else {
dsymlink := os.join_path(work_dir, 'dir_symlink') dsymlink := os.join_path_single(work_dir, 'dir_symlink')
os.symlink(work_dir, dsymlink) or { panic(err) } os.symlink(work_dir, dsymlink) or { panic(err) }
assert os.is_file(dsymlink) == false assert os.is_file(dsymlink) == false
} }
@ -128,7 +128,7 @@ fn test_is_file() {
$if windows { $if windows {
assert true assert true
} $else { } $else {
fsymlink := os.join_path(work_dir, 'file_symlink') fsymlink := os.join_path_single(work_dir, 'file_symlink')
os.symlink(tfile, fsymlink) or { panic(err) } os.symlink(tfile, fsymlink) or { panic(err) }
assert os.is_file(fsymlink) assert os.is_file(fsymlink)
} }
@ -309,47 +309,47 @@ fn test_cp() {
} }
fn test_mv() { fn test_mv() {
work_dir := os.join_path(os.getwd(), 'mv_test') work_dir := os.join_path_single(os.getwd(), 'mv_test')
os.mkdir_all(work_dir) or { panic(err) } os.mkdir_all(work_dir) or { panic(err) }
// Setup test files // Setup test files
tfile1 := os.join_path(work_dir, 'file') tfile1 := os.join_path_single(work_dir, 'file')
tfile2 := os.join_path(work_dir, 'file.test') tfile2 := os.join_path_single(work_dir, 'file.test')
tfile3 := os.join_path(work_dir, 'file.3') tfile3 := os.join_path_single(work_dir, 'file.3')
tfile_content := 'temporary file' tfile_content := 'temporary file'
os.write_file(tfile1, tfile_content) or { panic(err) } os.write_file(tfile1, tfile_content) or { panic(err) }
os.write_file(tfile2, tfile_content) or { panic(err) } os.write_file(tfile2, tfile_content) or { panic(err) }
// Setup test dirs // Setup test dirs
tdir1 := os.join_path(work_dir, 'dir') tdir1 := os.join_path_single(work_dir, 'dir')
tdir2 := os.join_path(work_dir, 'dir2') tdir2 := os.join_path_single(work_dir, 'dir2')
tdir3 := os.join_path(work_dir, 'dir3') tdir3 := os.join_path_single(work_dir, 'dir3')
os.mkdir(tdir1) or { panic(err) } os.mkdir(tdir1) or { panic(err) }
os.mkdir(tdir2) or { panic(err) } os.mkdir(tdir2) or { panic(err) }
// Move file with no extension to dir // Move file with no extension to dir
os.mv(tfile1, tdir1) or { panic(err) } os.mv(tfile1, tdir1) or { panic(err) }
mut expected := os.join_path(tdir1, 'file') mut expected := os.join_path_single(tdir1, 'file')
assert os.exists(expected) assert os.exists(expected)
assert !os.is_dir(expected) assert !os.is_dir(expected)
// Move dir with contents to other dir // Move dir with contents to other dir
os.mv(tdir1, tdir2) or { panic(err) } os.mv(tdir1, tdir2) or { panic(err) }
expected = os.join_path(tdir2, 'dir') expected = os.join_path_single(tdir2, 'dir')
assert os.exists(expected) assert os.exists(expected)
assert os.is_dir(expected) assert os.is_dir(expected)
expected = os.join_path(tdir2, 'dir', 'file') expected = os.join_path(tdir2, 'dir', 'file')
assert os.exists(expected) assert os.exists(expected)
assert !os.is_dir(expected) assert !os.is_dir(expected)
// Move dir with contents to other dir (by renaming) // Move dir with contents to other dir (by renaming)
os.mv(os.join_path(tdir2, 'dir'), tdir3) or { panic(err) } os.mv(os.join_path_single(tdir2, 'dir'), tdir3) or { panic(err) }
expected = tdir3 expected = tdir3
assert os.exists(expected) assert os.exists(expected)
assert os.is_dir(expected) assert os.is_dir(expected)
assert os.is_dir_empty(tdir2) assert os.is_dir_empty(tdir2)
// Move file with extension to dir // Move file with extension to dir
os.mv(tfile2, tdir2) or { panic(err) } os.mv(tfile2, tdir2) or { panic(err) }
expected = os.join_path(tdir2, 'file.test') expected = os.join_path_single(tdir2, 'file.test')
assert os.exists(expected) assert os.exists(expected)
assert !os.is_dir(expected) assert !os.is_dir(expected)
// Move file to dir (by renaming) // Move file to dir (by renaming)
os.mv(os.join_path(tdir2, 'file.test'), tfile3) or { panic(err) } os.mv(os.join_path_single(tdir2, 'file.test'), tfile3) or { panic(err) }
expected = tfile3 expected = tfile3
assert os.exists(expected) assert os.exists(expected)
assert !os.is_dir(expected) assert !os.is_dir(expected)
@ -375,7 +375,7 @@ fn test_cp_all() {
// regression test for executive runs with overwrite := true // regression test for executive runs with overwrite := true
os.cp_all('ex', './', true) or { panic(err) } os.cp_all('ex', './', true) or { panic(err) }
os.cp_all('ex', 'nonexisting', true) or { panic(err) } os.cp_all('ex', 'nonexisting', true) or { panic(err) }
assert os.exists(os.join_path('nonexisting', 'ex1.txt')) assert os.exists(os.join_path_single('nonexisting', 'ex1.txt'))
} }
fn test_realpath_of_empty_string_works() { fn test_realpath_of_empty_string_works() {
@ -398,7 +398,7 @@ fn test_realpath_non_existing() {
fn test_realpath_existing() { fn test_realpath_existing() {
existing_file_name := 'existing_file.txt' existing_file_name := 'existing_file.txt'
existing_file := os.join_path(os.temp_dir(), existing_file_name) existing_file := os.join_path_single(os.temp_dir(), existing_file_name)
os.rm(existing_file) or {} os.rm(existing_file) or {}
os.write_file(existing_file, 'abc') or {} os.write_file(existing_file, 'abc') or {}
assert os.exists(existing_file) assert os.exists(existing_file)
@ -843,7 +843,7 @@ fn test_expand_tilde_to_home() {
} }
fn test_execute() ? { fn test_execute() ? {
print0script := os.join_path(tfolder, 'print0.v') print0script := os.join_path_single(tfolder, 'print0.v')
// The output of the next command contains a 0 byte in the middle. // The output of the next command contains a 0 byte in the middle.
// Nevertheless, the execute function *should* return a string that // Nevertheless, the execute function *should* return a string that
// contains it. // contains it.

3
vlib/os/os_windows.c.v
View File

@ -503,7 +503,8 @@ pub fn is_writable_folder(folder string) ?bool {
if !is_dir(folder) { if !is_dir(folder) {
return error('`folder` is not a folder') return error('`folder` is not a folder')
} }
tmp_perm_check := join_path(folder, 'tmp_perm_check_pid_' + getpid().str()) tmp_folder_name := 'tmp_perm_check_pid_' + getpid().str()
tmp_perm_check := join_path_single(folder, tmp_folder_name)
mut f := open_file(tmp_perm_check, 'w+', 0o700) or { mut f := open_file(tmp_perm_check, 'w+', 0o700) or {
return error('cannot write to folder $folder: $err') return error('cannot write to folder $folder: $err')
} }

6
vlib/v/tests/valgrind/import_os_and_use_its_constants.v
View File

@ -7,8 +7,8 @@ fn main() {
println(os.args) println(os.args)
println(os.wd_at_startup) println(os.wd_at_startup)
// //
fullpath := os.join_path('abc', 'xyz', 'def') // fullpath := os.join_path('abc', 'xyz', 'def')
println(fullpath) // println(fullpath)
// //
x := 'abc' x := 'abc'
t := x.trim_right('/') t := x.trim_right('/')
@ -30,7 +30,7 @@ fn main() {
// exeparent_folder := os.dir(exe_realpath) // exeparent_folder := os.dir(exe_realpath)
// println(exeparent_folder) // println(exeparent_folder)
cdir := os.join_path(os.home_dir(), '.cache') cdir := os.join_path_single(os.home_dir(), '.cache')
println(cdir) println(cdir)
wd_realpath := os.real_path(os.wd_at_startup) wd_realpath := os.real_path(os.wd_at_startup)