diff --git a/TODOS.md b/TODOS.md index e1a6c21..6617afe 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2,6 +2,8 @@ 1. Consider giving db its own allocator +27. Commands are still leaking. + 2. Generate md and man pages again. 3. **db.odin:324-327** — Map iteration (`remote_set`) is non-deterministic. Same file can produce different JSON on each backup, causing spurious DB diffs. Sort remotes before storing. diff --git a/cli.odin b/cli.odin index f23b166..a13c54c 100644 --- a/cli.odin +++ b/cli.odin @@ -59,7 +59,7 @@ key somewhere, otherwise your data could be lost forever.`, parse_args :: proc(args: []string, out: io.Stream, err: io.Stream) -> (cmd: Command, ok: bool) { { cmd.out_buf = new(bufio.Writer) - bufio.writer_init(cmd.out_buf, out) + bufio.writer_init(cmd.out_buf, out, allocator = context.allocator) cmd.out = bufio.writer_to_writer(cmd.out_buf) cmd.err = err } @@ -256,9 +256,11 @@ has_flag :: proc(cmd: ^Command, name: string) -> bool { } delete_command :: proc(cmd: ^Command) { + bufio.writer_flush(cmd.out_buf) delete(cmd.args) delete(cmd.flags) delete(cmd.bool_set) bufio.writer_destroy(cmd.out_buf) free(cmd.out_buf) } + diff --git a/cmd_check.odin b/cmd_check.odin index b1b705e..3d926d1 100644 --- a/cmd_check.odin +++ b/cmd_check.odin @@ -54,6 +54,8 @@ cmd_check :: proc(cmd: ^Command) { if !list_ok { return } + defer delete(db_files) + defer for &file in db_files {delete_envfile(&file)} not_backed := find_unbacked(files_in_path[:], db_files[:]) @@ -61,13 +63,23 @@ cmd_check :: proc(cmd: ^Command) { if len(files_in_path) == 0 { fmt.wprintln(cmd.out, "No .env files found in the specified directory.", flush = false) } else { - fmt.wprintln(cmd.out, "✓ All .env files in the directory are backed up.", flush = false) + fmt.wprintln( + cmd.out, + "✓ All .env files in the directory are backed up.", + flush = false, + ) } } else { - fmt.wprintf(cmd.out, "Found %d .env file(s) that are not backed up:\n", len(not_backed), flush = false) + fmt.wprintf( + cmd.out, + "Found %d .env file(s) that are not backed up:\n", + len(not_backed), + flush = false, + ) for file in not_backed { fmt.wprintf(cmd.out, " %s\n", file, flush = false) } fmt.wprintln(cmd.out, "\nRun 'envr sync' to back up these files.", flush = false) } } + diff --git a/cmd_list.odin b/cmd_list.odin index 10fb49f..17157b2 100644 --- a/cmd_list.odin +++ b/cmd_list.odin @@ -26,6 +26,7 @@ cmd_list :: proc(cmd: ^Command) { return } defer delete(rows) + defer for &row in rows {delete_envfile(&row)} if terminal.is_terminal(os.stdout) { headers := []string{"Directory", "Path"} @@ -34,7 +35,7 @@ cmd_list :: proc(cmd: ^Command) { for row in rows { dir_str := strings.concatenate({row.Dir, "/"}, context.temp_allocator) filename := filepath.base(row.Path) - row_slice := make([]string, 2) + row_slice := make([]string, 2, context.temp_allocator) row_slice[0] = dir_str row_slice[1] = filename append(&table_rows, row_slice) diff --git a/config.odin b/config.odin index ea8225b..b313bad 100644 --- a/config.odin +++ b/config.odin @@ -220,7 +220,9 @@ envr_dir :: proc(config_path: string) -> string { return filepath.dir(config_path) } -data_path :: proc(config_path: string) -> string { - path, _ := filepath.join([]string{envr_dir(config_path), "data.envr"}) +// User is responsible for freeing the path +data_path :: proc(config_path: string, allocator := context.allocator) -> string { + path, _ := filepath.join([]string{envr_dir(config_path), "data.envr"}, allocator) return path } + diff --git a/db.odin b/db.odin index d360c2f..8ccd190 100644 --- a/db.odin +++ b/db.odin @@ -51,52 +51,51 @@ delete_envfile :: proc(f: ^EnvFile) { delete(f.contents) } -db_open :: proc(cfg_path: string) -> (Db, bool) { - cfg, ok := load_config(cfg_path) - if !ok { - return Db{}, false - } +db_open :: proc(cfg_path: string) -> (database: Db, ok: bool) { + database.cfg = load_config(cfg_path) or_return - data_path := data_path(cfg.config_path) - _, stat_err := os.stat(data_path, context.allocator) - - db: ^rawptr - rc := sqlite.db_open(":memory:", &db) - if rc != sqlite.OK { - fmt.printf("Error opening in-memory database: %s\n", sqlite.db_errmsg(db)) - return Db{}, false - } - - create_sql: cstring = "CREATE TABLE IF NOT EXISTS envr_env_files (path TEXT PRIMARY KEY NOT NULL, remotes TEXT, sha256 TEXT NOT NULL, contents TEXT NOT NULL)" - rc = sqlite.db_exec(db, create_sql, nil, nil, nil) - if rc != sqlite.OK { - fmt.printf("Error creating table: %s\n", sqlite.db_errmsg(db)) - sqlite.db_close(db) - return Db{}, false - } - - if stat_err == nil { - if !db_restore_from_encrypted(db, cfg) { - sqlite.db_close(db) - return Db{}, false + { + db: ^rawptr + rc := sqlite.db_open(":memory:", &db) + if rc != sqlite.OK { + fmt.printf("Error opening in-memory database: %s\n", sqlite.db_errmsg(db)) + return } + + create_sql: cstring = "CREATE TABLE IF NOT EXISTS envr_env_files (path TEXT PRIMARY KEY NOT NULL, remotes TEXT, sha256 TEXT NOT NULL, contents TEXT NOT NULL)" + rc = sqlite.db_exec(db, create_sql, nil, nil, nil) + if rc != sqlite.OK { + fmt.printf("Error creating table: %s\n", sqlite.db_errmsg(db)) + sqlite.db_close(db) + return + } + database.db = db } - return Db{db = db, cfg = cfg, changed = stat_err != nil}, true + // TODO: Use different allocators? + data_path := data_path(database.cfg.config_path, context.temp_allocator) + if os.exists(data_path) { + if ok = db_restore_from_encrypted(&database, data_path); !ok { + sqlite.db_close(database.db) + return + } + } else { + // DB was created + database.changed = true + } + + return database, true } -db_restore_from_encrypted :: proc(db: ^rawptr, cfg: Config) -> bool { - encrypted_data, read_err := os.read_entire_file_from_path( - data_path(cfg.config_path), - context.allocator, - ) +db_restore_from_encrypted :: proc(db: ^Db, data_path: string) -> bool { + encrypted_data, read_err := os.read_entire_file_from_path(data_path, context.allocator) defer delete(encrypted_data) if read_err != nil { fmt.printf("Error reading encrypted database: %v\n", read_err) return false } - plaintext, dec_ok := decrypt(encrypted_data, cfg.Keys[:]) + plaintext, dec_ok := decrypt(encrypted_data, db.cfg.Keys[:]) if !dec_ok { fmt.println("Error: decryption failed") return false @@ -112,7 +111,7 @@ db_restore_from_encrypted :: proc(db: ^rawptr, cfg: Config) -> bool { copy(buf[:len(plaintext)], plaintext) rc := sqlite.deserialize( - db, + db.db, "main", buf, n, @@ -121,7 +120,7 @@ db_restore_from_encrypted :: proc(db: ^rawptr, cfg: Config) -> bool { ) if rc != sqlite.OK { sqlite.free(buf) - fmt.printf("Error deserializing database: %s\n", sqlite.db_errmsg(db)) + fmt.printf("Error deserializing database: %s\n", sqlite.db_errmsg(db.db)) return false } @@ -130,6 +129,7 @@ db_restore_from_encrypted :: proc(db: ^rawptr, cfg: Config) -> bool { db_close :: proc(d: ^Db) { defer sqlite.db_close(d.db) + defer delete_config(&d.cfg) if d.changed { rc := sqlite.db_exec(d.db, "VACUUM", nil, nil, nil) @@ -566,6 +566,7 @@ string_to_cstring :: proc(s: string, allocator := context.allocator) -> cstring return cs } +// Caller is responsible for freeing the result clone_cstring :: proc(c: cstring, allocator := context.allocator) -> string { str, err := strings.clone_from_cstring(c, allocator) if err != nil { @@ -576,3 +577,4 @@ clone_cstring :: proc(c: cstring, allocator := context.allocator) -> string { return str } + diff --git a/db_test.odin b/db_test.odin index 217e758..bf6a91f 100644 --- a/db_test.odin +++ b/db_test.odin @@ -472,3 +472,51 @@ test_update_dir :: proc(t: ^testing.T) { testing.expect_value(t, f.Path, "/new/location/.env") } +@(test) +test_closing_db_has_no_leaks :: proc(t: ^testing.T) { + base := fmt.tprintf("/tmp/envr-test-leak-%d", os.get_pid()) + os.mkdir_all(base) + defer os.remove_all(base) + + cfg_path, err := filepath.join([]string{base, "config.json"}, context.temp_allocator) + testing.expect(t, err == nil, "cfgPath should build successfully") + + cfg := new_config([]string{"fixtures/keys/insecure-test-key"}, cfg_path) + defer delete_config(&cfg) + testing.expect(t, save_config(cfg, force = true), "save should succeed") + + db, ok := db_open(cfg_path) + testing.expect(t, ok, "db should open") + if !ok do return + db_close(&db) +} + +@(test) +test_open_existing_db_has_no_leaks :: proc(t: ^testing.T) { + base := fmt.tprintf("/tmp/envr-test-leak-existing-%d", os.get_pid()) + os.mkdir_all(base) + defer os.remove_all(base) + + cfg_path, err := filepath.join([]string{base, "config.json"}, context.temp_allocator) + testing.expect(t, err == nil, "cfgPath should build successfully") + + cfg := new_config([]string{"fixtures/keys/insecure-test-key"}, cfg_path) + defer delete_config(&cfg) + testing.expect(t, save_config(cfg, force = true), "save should succeed") + + // First open/close creates data.envr on disk + db, ok := db_open(cfg_path) + testing.expect(t, ok, "db should open") + if !ok do return + f := make_test_env_file("/project/.env", "abc123", "SECRET=value", []string{"git@github.com:user/repo.git"}) + defer delete(f.Remotes) + testing.expect(t, db_insert(&db, f), "insert should succeed") + db_close(&db) + + // Second open exercises db_restore_from_encrypted + db2, ok2 := db_open(cfg_path) + testing.expect(t, ok2, "db should open existing") + if !ok2 do return + db_close(&db2) +} + diff --git a/findr/walker.odin b/findr/walker.odin index f32e74f..f1d02e0 100644 --- a/findr/walker.odin +++ b/findr/walker.odin @@ -1,5 +1,6 @@ package findr +import "core:bytes" import "core:fmt" import "core:os" import "core:strings" @@ -54,19 +55,26 @@ Collector_Data :: struct { collect_worker :: proc(t: ^thread.Thread) { data := cast(^Collector_Data)t.data for { - batch, ok := chan.recv(data.ch) - if !ok do break + batch := chan.recv(data.ch) or_break + defer delete(batch) + start := 0 - for i in 0 ..< len(batch) { - if batch[i] == '\n' { - if i > start { - s, _ := strings.clone(string(batch[start:i])) - append(data.results, s) - } - start = i + 1 + for { + remaining: []u8 + #no_bounds_check {remaining = batch[start:]} + + idx := bytes.index_byte(remaining, '\n') + if idx < 0 do break + + i := start + idx + if i > start { + segment: []u8 + #no_bounds_check {segment = batch[start:i]} + s, _ := strings.clone(string(segment)) + append(data.results, s) } + start = i + 1 } - delete(batch) } } @@ -447,3 +455,4 @@ join_path :: proc(parent, child: string) -> string { copy(buf[pos:], child) return string(buf) } + diff --git a/main.odin b/main.odin index 5f63ed7..24cb238 100644 --- a/main.odin +++ b/main.odin @@ -1,14 +1,31 @@ package main -import "core:bufio" import "core:fmt" +import "core:mem" import "core:os" main :: proc() { + when ODIN_DEBUG { + heap_track: mem.Tracking_Allocator + mem.tracking_allocator_init(&heap_track, context.allocator) + defer mem.tracking_allocator_destroy(&heap_track) + defer if len(heap_track.allocation_map) > 0 { + for _, leak in heap_track.allocation_map { + fmt.eprintf("LEAK: %v leaked %m\n", leak.location, leak.size) + } + } + context.allocator = mem.tracking_allocator(&heap_track) + + temp_track: mem.Tracking_Allocator + mem.tracking_allocator_init(&temp_track, context.temp_allocator) + defer mem.tracking_allocator_destroy(&temp_track) + context.temp_allocator = mem.tracking_allocator(&temp_track) + } + defer free_all(context.temp_allocator) cmd, ok := parse_args(os.args, os.to_writer(os.stdout), os.to_writer(os.stderr)) - defer bufio.writer_flush(cmd.out_buf) + defer delete_command(&cmd) // delete flushes automatically if !ok { return }