Add Chmod() method to File#357
Open
owbone wants to merge 1 commit into
Open
Conversation
`os.File` offers a `Chmod()` method. This is often safer and more direct to use than `os.Chmod()` because it operates on an open file descriptor rather than having to lookup the file by name. Without this, it's possible for the target file to be renamed, in which case an `os.Chmod()` would either fail or apply to any file that's taken its place. Therefore, add the `Chmod()` method to the `File` interface, and implement it for all `File` implementations. The bulk of this change is in `MemMapFs`, which required moving the chmod functionality down into the `mem` package so that it can be shared between both `mem.File` and `MemMapFs`.
Owner
|
The underlying need is real and the implementation within this repo is correct. The problem is adding The right pattern — already established in this repo — is a narrower optional interface: // In symlink.go or a new chmod.go:
type ChmodFile interface {
Chmod(mode os.FileMode) error
}Callers that need it type-assert: if cf, ok := f.(afero.ChmodFile); ok {
return cf.Chmod(mode)
}
return afero.ErrNoChmod // or similarThis keeps the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
os.Fileoffers aChmod()method. This is often safer and more directto use than
os.Chmod()because it operates on an open file descriptorrather than having to lookup the file by name. Without this, it's
possible for the target file to be renamed, in which case an
os.Chmod()would either fail or apply to any file that's taken itsplace.
Therefore, add the
Chmod()method to theFileinterface, andimplement it for all
Fileimplementations. The bulk of this change isin
MemMapFs, which required moving the chmod functionality down intothe
mempackage so that it can be shared between bothmem.FileandMemMapFs.