Skip to content
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

Fixes #32 #49

Merged
merged 12 commits into from
Oct 28, 2021
Merged

Fixes #32 #49

merged 12 commits into from
Oct 28, 2021

Conversation

vhqtvn
Copy link
Contributor

@vhqtvn vhqtvn commented Oct 21, 2021

Fixes #32

An error might not contain xwhat, only what. This fix assumes that these errors are not a leaking error.

An example one in my case:

<error>
  <unique>0x0</unique>
  <tid>1</tid>
  <kind>SyscallParam</kind>
  <what>Syscall param statx(file_name) points to unaddressable byte(s)</what>
  <stack>REMOVED</stack>
  <auxwhat>Address 0x0 is not stack'd, malloc'd or (recently) free'd</auxwhat>
</error>

Copy link
Owner

@jfrimmel jfrimmel left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! Just ignore the failing clippy CI test.

I just have a nitpick and a question: do you basically change the code to always have an xwhat-error, which is possibly empty?

Comment on lines 80 to 81
.map(|o_: xml::Output| {
let mut o = o_;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
.map(|o_: xml::Output| {
let mut o = o_;
.map(|o: xml::Output| {

Furthermore, couldn you please rename all os to output?

@vhqtvn
Copy link
Contributor Author

vhqtvn commented Oct 26, 2021

Yes i took a quick look on serde and cant find a quick way to case-typing parsing so i just let it to be empty on failure. And then in the reporter I let it filter out the errors that doesnt have leaking info (filter(|e| e.resources.bytes > 0 || e.resources.blocks > 0)).

@jfrimmel jfrimmel merged commit 8de40ea into jfrimmel:master Oct 28, 2021
@jfrimmel
Copy link
Owner

Thank you for your contribution, I'll make a release soon.

@Anders429
Copy link

@jfrimmel Any chance we could get a release on crates.io that includes this change? I've had this issue come up for me, and it looks like a new release with this PR included would fix it :)

@jfrimmel
Copy link
Owner

Oh, well, I've simply forgot doing it... I'll need an automatic release workflow in this repository...

I've published version 2.0.2 containing this fix.

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.

Not working on unit tests (missing field xwhat)
3 participants