Closed EyitopeIO closed 1 month ago
If the user-specified cache directory had other content the user cared about, then clearing the cache would wipe that directory along with the non-httpdirfs content.
Perhaps the help should be updated with this comment?
Yeh, your previous pull request successfully address the stated GitHub issue, so I merged it. If you cherry-pick this on top of the current master, update the pull request saying that it fixes #154, I will merge it if it is up to standard like the previous one.
You need to rebase your changes on top the head of the master branch. There are merge conflicts. The quickest way to resolve it is to rebase. I was happy with your previous commit, that's why I merged it. :)
Commenting as @EyitopeIO
Good to know. Thanks.
I'm testing with $HOME
non-existent, but got a crash. I'd do my best to keep the standard up and request a re-review as soon as it's ready.
I think I don't have permissions to edit the PR title because I don't see the edit button. I see you may be able to change the PR title at the point of merging.
In general you need to take more care about your variable names, I think.
Please address all the code quality issues introduced by your change flagged up by the static analysers.
Also, in general, when you have a if statement, you should condition them in such a way that the most common condition occurs at the first branch, so the code is easier to read. This is what I want in this repository for new contributions.
For example, I prefer:
if (common_condition) {
// handle common condition
} else {
// handle unexpected condition
}
I don't like:
if (!common_condition) {
// handle unexpected condition
} else {
//deal with common condition
}
Anyways, this pull request is heading towards a good direction. I like it so far.
Comments on static analyser
realpath
function are false-positives because the function is already called with a destination buffer set to NULL.$HOME
variable being modified by an attacker is not applicable. We assume the user's system is already secure, so whatever is in$HOME
is legit.Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Fixes https://github.com/fangfufu/httpdirfs/issues/154