From 390394b56ba91beb9b9adc66838e7c62239502bb Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 22 Jul 2019 16:51:33 +0200 Subject: [PATCH] string: make substr() copy the data, like in Java and C#; remove .cstr() this makes managing memory used by strings much easier V strings are now fully compatible with C strings --- compiler/scanner.v | 2 +- vlib/builtin/string.v | 27 ++++++++++------- vlib/http/download_nix.v | 4 +-- vlib/http/http_nix.v | 10 +++---- vlib/net/socket.v | 2 +- vlib/net/socket_test.v | 2 +- vlib/os/os.v | 65 +++++++++++++++++++--------------------- vlib/os/os_win.v | 2 +- vlib/pg/pg.v | 4 +-- vlib/szip/szip.v | 16 +++++----- 10 files changed, 68 insertions(+), 66 deletions(-) diff --git a/compiler/scanner.v b/compiler/scanner.v index f1e8555e8e..b156bfd2c9 100644 --- a/compiler/scanner.v +++ b/compiler/scanner.v @@ -43,7 +43,7 @@ fn new_scanner(file_path string) *Scanner { // BOM check if raw_text.len >= 3 { - c_text := raw_text.cstr() + c_text := raw_text.str if c_text[0] == 0xEF && c_text[1] == 0xBB && c_text[2] == 0xBF { // skip three BOM bytes diff --git a/vlib/builtin/string.v b/vlib/builtin/string.v index 64c84804b3..4ae63877dd 100644 --- a/vlib/builtin/string.v +++ b/vlib/builtin/string.v @@ -70,10 +70,12 @@ pub fn (a string) clone() string { return b } +/* pub fn (s string) cstr() byteptr { clone := s.clone() return clone.str } +*/ pub fn (s string) replace(rep, with string) string { if s.len == 0 || rep.len == 0 { @@ -332,16 +334,7 @@ pub fn (s string) right(n int) string { return s.substr(n, s.len) } -// Because the string is immutable, it is safe for multiple strings to share -// the same storage, so slicing s results in a new 2-word structure with a -// potentially different pointer and length that still refers to the same byte -// sequence. This means that slicing can be done without allocation or copying, -// making string slices as efficient as passing around explicit indexes. -// substr without allocations. Reuses memory and works great. BUT. This substring does not have -// a \0 at the end, and it's not possible to add it. So if we have s = 'privet' -// and substr := s.substr_fast(1, 4) ('riv') -// puts(substr.str) will print 'rivet' -// Avoid using C functions with these substrs! +// substr pub fn (s string) substr(start, end int) string { /* if start > end || start >= s.len || end > s.len || start < 0 || end < 0 { @@ -353,11 +346,25 @@ pub fn (s string) substr(start, end int) string { return '' } len := end - start + + // Copy instead of pointing, like in Java and C#. + // Much easier to free such strings. + mut res := string { + len: len + str: malloc(len + 1) + } + for i := 0; i < len; i++ { + res.str[i] = s.str[start + i] + } + res.str[len] = `\0` + return res +/* res := string { str: s.str + start len: len } return res +*/ } // KMP search diff --git a/vlib/http/download_nix.v b/vlib/http/download_nix.v index 3a989f480b..8a5ffbec5b 100644 --- a/vlib/http/download_nix.v +++ b/vlib/http/download_nix.v @@ -29,9 +29,9 @@ fn download_file_with_progress(url, out string, cb downloadfn, cb_finished downl if isnil(curl) { return } - cout := out.cstr() + cout := out.str fp := C.fopen(cout, 'wb') - C.curl_easy_setopt(curl, CURLOPT_URL, url.cstr()) + C.curl_easy_setopt(curl, CURLOPT_URL, url.str) C.curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, download_cb) data := &DownloadStruct { stream:fp diff --git a/vlib/http/http_nix.v b/vlib/http/http_nix.v index 0b6f1dc940..1b07175476 100644 --- a/vlib/http/http_nix.v +++ b/vlib/http/http_nix.v @@ -104,7 +104,7 @@ pub fn (req &Request) do() Response { } // options // url2 := req.url.clone() - C.curl_easy_setopt(curl, CURLOPT_URL, req.url.cstr())// ..clone()) + C.curl_easy_setopt(curl, CURLOPT_URL, req.url.str)// ..clone()) // C.curl_easy_setopt(curl, CURLOPT_URL, 'http://example.com') // return resp // curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0); @@ -117,7 +117,7 @@ pub fn (req &Request) do() Response { C.curl_easy_setopt(curl, CURLOPT_HEADERDATA, &hchunk) C.curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1) if req.typ == 'POST' { - C.curl_easy_setopt(curl, CURLOPT_POSTFIELDS, req.data.cstr()) + C.curl_easy_setopt(curl, CURLOPT_POSTFIELDS, req.data.str) C.curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, 'POST') // req.headers << 'Content-Type: application/x-www-form-urlencoded' } @@ -126,7 +126,7 @@ pub fn (req &Request) do() Response { // for i, h := range req.headers { for key, val in req.headers { h := '$key: $val' - hlist = C.curl_slist_append(hlist, h.cstr()) + hlist = C.curl_slist_append(hlist, h.str) } // curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, // (long)CURL_HTTP_VERSION_2TLS);�`C�ʀ9� C.curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1) @@ -190,11 +190,11 @@ pub fn (req &Request) do() Response { } fn unescape(s string) string { - return string(byteptr(C.curl_unescape(s.cstr(), s.len))) + return string(byteptr(C.curl_unescape(s.str, s.len))) } fn escape(s string) string { - return string(byteptr(C.curl_escape(s.cstr(), s.len))) + return string(byteptr(C.curl_escape(s.str, s.len))) } // //////////////// diff --git a/vlib/net/socket.v b/vlib/net/socket.v index 0f148d89e8..2be8db1504 100644 --- a/vlib/net/socket.v +++ b/vlib/net/socket.v @@ -159,7 +159,7 @@ pub fn (s Socket) connect(address string, port int) int { info := &C.addrinfo{!} sport := '$port' - info_res := C.getaddrinfo(address.cstr(), sport.cstr(), &hints, &info) + info_res := C.getaddrinfo(address.str, sport.str, &hints, &info) if info_res != 0 { println('socket: getaddrinfo failed') return info_res diff --git a/vlib/net/socket_test.v b/vlib/net/socket_test.v index 6c023fc614..26ab544dfc 100644 --- a/vlib/net/socket_test.v +++ b/vlib/net/socket_test.v @@ -9,7 +9,7 @@ fn test_socket() { // println(socket) // message := 'Hello World' -// socket.send(message.cstr(), message.len) +// socket.send(message.str, message.len) // println('Sent: ' + message) // bytes := client.recv(1024) diff --git a/vlib/os/os.v b/vlib/os/os.v index 49f1a0b083..f4ee0a38df 100644 --- a/vlib/os/os.v +++ b/vlib/os/os.v @@ -106,8 +106,7 @@ fn parse_windows_cmd_line(cmd byteptr) []string { // read_file reads the file in `path` and returns the contents. pub fn read_file(path string) ?string { mut mode := 'rb' - cpath := path.cstr() - fp := C.fopen(cpath, mode.cstr()) + fp := C.fopen(path.str, mode.str) if isnil(fp) { return error('failed to open file "$path"') } @@ -130,7 +129,7 @@ pub fn file_size(path string) int { } pub fn mv(old, new string) { - C.rename(old.cstr(), new.cstr()) + C.rename(old.str, new.str) } // read_lines reads the file in `path` into an array of lines. @@ -138,8 +137,7 @@ pub fn mv(old, new string) { pub fn read_lines(path string) []string { mut res := []string mut buf := [1000]byte - cpath := path.cstr() - fp := C.fopen(cpath, 'rb') + fp := C.fopen(path.str, 'rb') if isnil(fp) { // TODO // return error('failed to open file "$path"') @@ -171,9 +169,8 @@ fn read_ulines(path string) []ustring { } pub fn open(path string) ?File { - cpath := path.cstr() file := File { - cfile: C.fopen(cpath, 'rb') + cfile: C.fopen(path.str, 'rb') } if isnil(file.cfile) { return error('failed to open file "$path"') @@ -183,9 +180,8 @@ pub fn open(path string) ?File { // create creates a file at a specified location and returns a writable `File` object. pub fn create(path string) ?File { - cpath := path.cstr() file := File { - cfile: C.fopen(cpath, 'wb') + cfile: C.fopen(path.str, 'wb') } if isnil(file.cfile) { return error('failed to create file "$path"') @@ -194,9 +190,8 @@ pub fn create(path string) ?File { } pub fn open_append(path string) ?File { - cpath := path.cstr() file := File { - cfile: C.fopen(cpath, 'ab') + cfile: C.fopen(path.str, 'ab') } if isnil(file.cfile) { return error('failed to create file "$path"') @@ -205,8 +200,8 @@ pub fn open_append(path string) ?File { } pub fn (f File) write(s string) { - ss := s.clone() - C.fputs(ss.cstr(), f.cfile) + ss := s.clone() // TODO is clone() needed here? + C.fputs(ss.str, f.cfile) // ss.free() // C.fwrite(s.str, 1, s.len, f.cfile) } @@ -228,7 +223,7 @@ pub fn (f File) writeln(s string) { // C.fwrite(s.str, 1, s.len, f.cfile) // ss := s.clone() // TODO perf - C.fputs(s.cstr(), f.cfile) + C.fputs(s.str, f.cfile) // ss.free() C.fputs('\n', f.cfile) } @@ -243,7 +238,7 @@ pub fn (f File) close() { // system starts the specified command, waits for it to complete, and returns its code. pub fn system(cmd string) int { - ret := C.system(cmd.cstr()) + ret := C.system(cmd.str) if ret == -1 { os.print_c_errno() } @@ -251,7 +246,7 @@ pub fn system(cmd string) int { } fn popen(path string) *FILE { - cpath := path.cstr() + cpath := path.str $if windows { return C._popen(cpath, 'r') } @@ -279,7 +274,7 @@ pub fn exec(cmd string) string { // `getenv` returns the value of the environment variable named by the key. pub fn getenv(key string) string { - s := C.getenv(key.cstr()) + s := C.getenv(key.str) if isnil(s) { return '' } @@ -291,13 +286,13 @@ pub fn setenv(name string, value string, overwrite bool) int { format := '$name=$value' if overwrite { - return C._putenv(format.cstr()) + return C._putenv(format.str) } return -1 } $else { - return C.setenv(name.cstr(), value.cstr(), overwrite) + return C.setenv(name.str, value.str, overwrite) } } @@ -305,10 +300,10 @@ pub fn unsetenv(name string) int { $if windows { format := '${name}=' - return C._putenv(format.cstr()) + return C._putenv(format.str) } $else { - return C.unsetenv(name.cstr()) + return C.unsetenv(name.str) } } @@ -322,7 +317,7 @@ pub fn file_exists(path string) bool { pub fn dir_exists(path string) bool { $if windows { - attr := int(C.GetFileAttributes(path.cstr())) + attr := int(C.GetFileAttributes(path.str)) if attr == INVALID_FILE_ATTRIBUTES { return false } @@ -332,7 +327,7 @@ pub fn dir_exists(path string) bool { return false } $else { - dir := C.opendir(path.cstr()) + dir := C.opendir(path.str) res := !isnil(dir) if res { C.closedir(dir) @@ -345,27 +340,27 @@ pub fn dir_exists(path string) bool { pub fn mkdir(path string) { $if windows { path = path.replace('/', '\\') - C.CreateDirectory(path.cstr(), 0) + C.CreateDirectory(path.str, 0) } $else { - C.mkdir(path.cstr(), 511)// S_IRWXU | S_IRWXG | S_IRWXO + C.mkdir(path.str, 511)// S_IRWXU | S_IRWXG | S_IRWXO } } // rm removes file in `path`. pub fn rm(path string) { - C.remove(path.cstr()) - // C.unlink(path.cstr()) + C.remove(path.str) + // C.unlink(path.str) } // rmdir removes a specified directory. pub fn rmdir(path string) { $if !windows { - C.rmdir(path.cstr()) + C.rmdir(path.str) } $else { - C.RemoveDirectoryA(path.cstr()) + C.RemoveDirectoryA(path.str) } } @@ -594,7 +589,7 @@ pub fn is_dir(path string) bool { } $else { statbuf := C.stat{} - cstr := path.cstr() + cstr := path.str if C.stat(cstr, &statbuf) != 0 { return false } @@ -604,10 +599,10 @@ pub fn is_dir(path string) bool { pub fn chdir(path string) { $if windows { - C._chdir(path.cstr()) + C._chdir(path.str) } $else { - C.chdir(path.cstr()) + C.chdir(path.str) } } @@ -657,7 +652,7 @@ pub fn ls(path string) []string { mut find_file_data := win32finddata{} mut dir_files := []string // We can also check if the handle is valid. but using dir_exists instead - // h_find_dir := C.FindFirstFile(path.cstr(), &find_file_data) + // h_find_dir := C.FindFirstFile(path.str, &find_file_data) // if (INVALID_HANDLE_VALUE == h_find_dir) { // return dir_files // } @@ -671,7 +666,7 @@ pub fn ls(path string) []string { path_files := '$path\\*' // NOTE:TODO: once we have a way to convert utf16 wide character to utf8 // we should use FindFirstFileW and FindNextFileW - h_find_files := C.FindFirstFile(path_files.cstr(), &find_file_data) + h_find_files := C.FindFirstFile(path_files.str, &find_file_data) first_filename := tos(&find_file_data.cFileName, strlen(find_file_data.cFileName)) if first_filename != '.' && first_filename != '..' { dir_files << first_filename @@ -687,7 +682,7 @@ pub fn ls(path string) []string { } $else { mut res := []string - dir := C.opendir(path.cstr()) + dir := C.opendir(path.str) if isnil(dir) { println('ls() couldnt open dir "$path"') print_c_errno() diff --git a/vlib/os/os_win.v b/vlib/os/os_win.v index 9a0296b59c..02a49b6e88 100644 --- a/vlib/os/os_win.v +++ b/vlib/os/os_win.v @@ -15,7 +15,7 @@ type HANDLE voidptr // get_file_handle retrieves the operating-system file handle that is associated with the specified file descriptor. pub fn get_file_handle(path string) HANDLE { mode := 'rb' - _fd := C.fopen(path.cstr(), mode.cstr()) + _fd := C.fopen(path.str, mode.str) if _fd == 0 { return HANDLE(INVALID_HANDLE_VALUE) } diff --git a/vlib/pg/pg.v b/vlib/pg/pg.v index fa0502fc72..d5d60f90df 100644 --- a/vlib/pg/pg.v +++ b/vlib/pg/pg.v @@ -29,7 +29,7 @@ fn C.PQgetvalue(voidptr, int, int) byteptr pub fn connect(dbname, user string) DB { conninfo := 'host=localhost user=$user dbname=$dbname' - conn:=C.PQconnectdb(conninfo.cstr()) + conn:=C.PQconnectdb(conninfo.str) status := C.PQstatus(conn) if status != CONNECTION_OK { error_msg := C.PQerrorMessage(conn) @@ -87,7 +87,7 @@ pub fn (db DB) q_strings(query string) []pg.Row { } pub fn (db DB) exec(query string) []pg.Row { - res := C.PQexec(db.conn, query.cstr()) + res := C.PQexec(db.conn, query.str) e := string(C.PQerrorMessage(db.conn)) if e != '' { println('pg exec error:') diff --git a/vlib/szip/szip.v b/vlib/szip/szip.v index 237e101703..de04ef3839 100644 --- a/vlib/szip/szip.v +++ b/vlib/szip/szip.v @@ -53,8 +53,8 @@ pub fn open(name string, level int, mode string) ?zip_ptr { if mode != M_WRITE && mode != M_RONLY && mode != M_APPEND { return error('szip: invalid provided open mode') } - /* struct zip_t* */_p_zip := zip_ptr(C.zip_open(name.cstr(), - _nlevel, mode.cstr())) + /* struct zip_t* */_p_zip := zip_ptr(C.zip_open(name.str, + _nlevel, mode.str)) if _p_zip == zip_ptr(0) { return error('szip: cannot open/create/append new zip archive.') } @@ -83,7 +83,7 @@ pub fn (z mut zip_ptr) close() { * @return the return code - 0 on success, negative number (< 0) on error. */ pub fn (zentry mut zip_ptr) open_entry(name string) /*?*/bool { - res := C.zip_entry_open(zentry, name.cstr()) + res := C.zip_entry_open(zentry, name.str) return res != -1 } @@ -203,7 +203,7 @@ pub fn (zentry mut zip_ptr) write_entry(data []byte) bool { * @return the return code - 0 on success, negative number (< 0) on error. */ pub fn (zentry mut zip_ptr) create_entry(name string) bool { - res := C.zip_entry_fwrite(zentry, name.cstr()) + res := C.zip_entry_fwrite(zentry, name.str) return res == 0 } @@ -241,11 +241,11 @@ pub fn (zentry mut zip_ptr) read_entry() ?voidptr { * @return the return code - 0 on success, negative number (< 0) on error. */ pub fn (zentry mut zip_ptr) extract_entry(path string) /*?*/bool { - if C.access(path.cstr(), 0) == -1 { + if C.access(path.str, 0) == -1 { return false //return error('Cannot open file for extracting, file not exists') } - res := C.zip_entry_fread(zentry, path.cstr()) + res := C.zip_entry_fread(zentry, path.str) return res == 0 } @@ -260,11 +260,11 @@ pub fn (zentry mut zip_ptr) extract_entry(path string) /*?*/bool { * @return the return code - 0 on success, negative number (< 0) on error. */ /*fn (zentry mut zip_ptr) extract(path string) bool { - if C.access(path.cstr(), 0) == -1 { + if C.access(path.str, 0) == -1 { return false //return error('Cannot open directory for extracting, directory not exists') } - res := C.zip_extract(zentry, path.cstr(), 0, 0) + res := C.zip_extract(zentry, path.str, 0, 0) return res == 0 }*/