fix: stream file rewrites with bounded memory#88
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be1b89a6eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| split := len(data) - overlap | ||
| process := data[:split] | ||
| rest = append([]byte(nil), data[split:]...) |
There was a problem hiding this comment.
For non-final chunks, process still includes the byte at len(data)-len(find), so an occurrence that ends exactly at the chunk boundary is split after its first byte and can never be matched. For example, with a 6-byte find ending at offset 256KiB, the first byte is written in this chunk and the remaining 5 bytes are carried, leaving the occurrence unchanged and possibly returning changed=false if it was the only match. The streamed replacer needs to keep every byte that could begin a match until the next pass.
Useful? React with 👍 / 👎.
| tempName := filepath.Join(f.Dir(), RandomString(20)) | ||
| out, err := os.OpenFile(tempName, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) |
There was a problem hiding this comment.
This opens the randomly named temp path with O_TRUNC before knowing whether the file contains a match, so if the generated name already exists in the target directory, a no-op replacement can truncate that unrelated file and then remove it at the !changed cleanup path. The previous ReplaceContents returned before creating any temp file when there was no match; use an exclusive temp-file primitive such as os.CreateTemp/O_EXCL, and preferably defer temp creation until a replacement is actually needed.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #8 and #14. Rewrites use a bounded streaming buffer instead of loading entire files into memory, and only write when replacements actually occur.
Test plan
go test ./...Made with Cursor