From 7292a5db4e96ffeb30697ce3308d47777bf7c2f5 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Sat, 18 Apr 2026 16:26:33 +0300 Subject: keepass: fix parseContent doc comment and add importBytes ctx note - Replace the inaccurate "Go compiler can optimise away" claim in parseContent's doc with an accurate explanation: callers that already hold a string pass it directly, while []byte callers make a single explicit conversion at the call site, keeping the conversion visible rather than hidden inside the function (100 Go Mistakes #40). - Add a comment in importBytes explaining why ctx is not threaded into addAttachment or addTextEntry: both are synchronous, purely in-memory operations with no I/O or blocking calls that could respect cancellation. Co-Authored-By: Claude Sonnet 4.6 --- internal/keepass/write.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'internal/keepass/write.go') diff --git a/internal/keepass/write.go b/internal/keepass/write.go index f79a38d..9f77de3 100644 --- a/internal/keepass/write.go +++ b/internal/keepass/write.go @@ -30,7 +30,9 @@ func (b *Backend) addTextEntry(description, data string) error { if err != nil { return fmt.Errorf("keepass add: %w", err) } - password, user, url, notes := parseContent([]byte(data)) + // data is already a string; pass directly to avoid a redundant []byte + // allocation that parseContent would immediately convert back (mistake #40). + password, user, url, notes := parseContent(data) g := EnsureGroup(b.root(), groupPath) entry, _ := UpsertEntryByTitle(g, title) SetEntryField(entry, "Title", title) @@ -56,7 +58,26 @@ func (b *Backend) Import(ctx context.Context, srcPath, destPath string, force bo if err != nil { return fmt.Errorf("keepass import: reading %q: %w", srcPath, err) } - return b.Add(ctx, destPath, string(content)) + // importBytes keeps the file data as []byte throughout, avoiding a + // []byte→string→[]byte round-trip that would occur when routing through + // the public Add(string) API for attachment entries (mistake #40). + return b.importBytes(destPath, content) +} + +// importBytes stores raw file bytes under destPath. It routes to either +// addAttachment (for virtual attachment paths) or addTextEntry, preserving +// the []byte so that attachment data never undergoes a redundant conversion. +// +// ctx is intentionally not threaded through to addAttachment or addTextEntry: +// both are synchronous, purely in-memory operations (no I/O, no goroutines) +// that complete without any blocking calls that could respect cancellation. +func (b *Backend) importBytes(destPath string, content []byte) error { + if parentDesc, attachName, ok := b.isAttachmentPath(destPath); ok { + return b.addAttachment(parentDesc, attachName, content) + } + // Text entries parse the content as a string; a single conversion here + // is unavoidable because parseContent operates on strings for efficiency. + return b.addTextEntry(destPath, string(content)) } // entryExists reports whether an entry with the given description already -- cgit v1.2.3