Skip to content

Conversation

hj-johannes-lee
Copy link
Contributor

Plugin used to consider only the value "-1" but there are some cases when files show "\n" or "\n\x00". This makes plugin to have wrong status of the device. So, trim the value after \n so only numerical value can be read.

@hj-johannes-lee hj-johannes-lee requested a review from mythi as a code owner August 5, 2024 12:22
@tkatila
Copy link
Contributor

tkatila commented Aug 5, 2024

@hj-johannes-lee please rebase for the IAA fix.

Plugin used to consider only the value "-1" but there are some
cases when files show "\n" or "\n\x00". This makes plugin to have
wrong status of the device. So, trim the value after \n so only
numerical value can be read.

Signed-off-by: Hyeongju Johannes Lee <[email protected]>
@hj-johannes-lee
Copy link
Contributor Author

@tkatila done!

@tkatila tkatila merged commit 2560bc0 into intel:main Aug 6, 2024
Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be simpler just to trim the string: https://pkg.go.dev/strings#TrimSpace ?

@mythi
Copy link
Contributor

mythi commented Aug 19, 2024

Wouldn't it be simpler just to trim the string: https://pkg.go.dev/strings#TrimSpace ?

it does not give the expected result:

        fmt.Println("0" == strings.TrimSpace(string(contents)))
        fmt.Println("0" == strings.TrimRight(strings.TrimSpace(string(contents)), "\n"))
        fmt.Println("0" == strings.TrimRight(string(contents), "\n\x00"))

gives:

false
false
true

@eero-t
Copy link
Contributor

eero-t commented Aug 19, 2024

strings.Trim(string(contents), "\n\x00"))?

@mythi
Copy link
Contributor

mythi commented Aug 19, 2024

strings.Trim(string(contents), "\n\x00"))?

Split() was chosen because it can work with and without \x00. The observation was that debugfsfile attribute reads give data with and without the null char. If this is changed by the kernel at some point. the code here would still work.(FWIW, later I also testedstrconv.ParseInt()` to compare against zero. That could be an alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants