-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix failing test and the false negative CI #19
Conversation
@@ -1,3 +1,4 @@ | |||
.SHELLFLAGS = -ec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 isn't that usually the default? Is that some weird configuration in GH Actions?
edit: ah! interesting https://www.gnu.org/software/make/manual/html_node/Choosing-the-Shell.html
The default value of
.SHELLFLAGS
is-c
normally, or-ec
in POSIX-conforming mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@kolyshkin PTAL
Would setting |
I just know this |
mountinfo/mountinfo_linux.go
Outdated
@@ -194,7 +194,7 @@ func unescape(path string) (string, error) { | |||
v := c - '0' | |||
for j := 2; j < 4; j++ { // one digit already; two more | |||
x := s[j] - '0' | |||
if x < 0 || x > 7 { | |||
if x > 7 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I think the correct thing to do would be
if s[j] < '0' || s[j] > '7' {
return "", fmt.Errorf("bad escape sequence %q: not a digit", s[:3])
}
x := s[j] - '0'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't fix anything (we will still catch underflow in case s[j] is less than '0'), but it's a tad cleaner code with no ambiguity.
Thank you for the fixes! Can you please split it into 3 commits:
with a brief commit message describing the reason for the change. Otherwise it won't be easy to follow the git history |
Add -e to .SHELLFLAGS, so for loop can fail correctly Signed-off-by: Shengjing Zhu <zhsj@debian.org>
Align the test result after: 0b171c2 mountinfo: also unescape fstype and source Signed-off-by: Shengjing Zhu <zhsj@debian.org>
The old check is always true when comparing with 0, since x is byte type Signed-off-by: Shengjing Zhu <zhsj@debian.org>
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@kolyshkin PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.