tail: Do not panic in path.is_tailable() check, but return false instead#3919
tail: Do not panic in path.is_tailable() check, but return false instead#3919Joining7943 wants to merge 2 commits intouutils:mainfrom
Conversation
|
I wasn't able to reproduce the bad file descriptor error. However, I've added debug output in case we're running into the todo(), so maybe we have chance to figure out what's going wrong. |
| /// Return true if `path` is is a file type that can be tailed | ||
| fn is_tailable(&self) -> bool { | ||
| self.is_file() || self.exists() && self.metadata().unwrap().is_tailable() | ||
| self.is_file() || self.exists() && self.metadata().map_or(false, |meta| meta.is_tailable()) |
There was a problem hiding this comment.
The call to metadata() will return an error if user lacks permissions or the path does not exist. However, is_file() and exists() already return false if user cannot access metadata. So this unwrap should never be triggered.
There was a problem hiding this comment.
But that's what happened
There was a problem hiding this comment.
Then there could be an issue with time-of-check / time-of-use. You could try try_exists() instead of exists(), but it's only available in rust 1.63
There was a problem hiding this comment.
Why do you want to stick to unwrap()?
There was a problem hiding this comment.
Silencing this error doesn't feel right.
This unwrap triggers, but (according to documentation) it shouldn't, which means there's an issue somewhere up in the call stack.
There was a problem hiding this comment.
It's not about silencing the error but instead returning such a path as untailable, that's what the signature and name of the method states. No surprises.
There was a problem hiding this comment.
Why not just
fn is_tailable(&self) -> bool {
self.metadata().map_or(false, |meta| meta.is_tailable())
}
|
Sorry but it is now conflicting |
|
No problem :) |
|
@Joining7943 should we abandon this PR? |
|
I'll close this one. |
4 participants
I'm just trying something here to deal with the spontanously occurring stdin bad file descriptor error on macos.