Conversation
Signed-off-by: Tiger <rbalajis25@gmail.com>
Stebalien
left a comment
There was a problem hiding this comment.
A couple of nits but otherwise LGTM.
repo/fsrepo/fsrepo.go
Outdated
| f, err := os.Create(filepath.Join(r.path, apiFile)) | ||
| // Create a temp file to write the address, so that we don't leave empty file when the | ||
| // program crashes after creating the file. | ||
| f, err := os.Create(filepath.Join(r.path, apiFile+"-tmp")) |
There was a problem hiding this comment.
I'd name this something like:
| f, err := os.Create(filepath.Join(r.path, apiFile+"-tmp")) | |
| f, err := os.Create(filepath.Join(r.path, "." + apiFile+".tmp")) |
repo/fsrepo/fsrepo.go
Outdated
| } | ||
|
|
||
| // Atomically rename the temp file to the correct file name. | ||
| return os.Rename(filepath.Join(r.path, apiFile+"-tmp"), filepath.Join(r.path, apiFile)) |
There was a problem hiding this comment.
| return os.Rename(filepath.Join(r.path, apiFile+"-tmp"), filepath.Join(r.path, apiFile)) | |
| return os.Rename(filepath.Join(r.path, "." + apiFile+".tmp"), filepath.Join(r.path, apiFile)) |
| @@ -359,14 +359,22 @@ func (r *FSRepo) Path() string { | |||
|
|
|||
| // SetAPIAddr writes the API Addr to the /api file. | |||
| func (r *FSRepo) SetAPIAddr(addr ma.Multiaddr) error { | |||
There was a problem hiding this comment.
If we fail to rename the temporary file, let's remove it.
There was a problem hiding this comment.
I would like to keep as it is. It feels like too much defensive programming.
When users run the program next time, newly created tmp will override the existing tmp file.
If you like to remove, I'll add code to remove the tmp file if rename fails
There was a problem hiding this comment.
Well, it is defensive programming. While this is unlikely to fail on Linux, renaming can fail on Windows if some other user is accessing the API file at the same time. Yes, we'll end up deleting the temporary file later, but that isn't a reason to not at least try to clean it up early.
There was a problem hiding this comment.
Made changes. Thanks for the explaining the reason.
Signed-off-by: Tiger <rbalajis25@gmail.com>
Signed-off-by: Tiger <rbalajis25@gmail.com>
Fix: #7227
Signed-off-by: Tiger rbalajis25@gmail.com