Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

Add Snappy package #949

Merged
merged 1 commit into from
Aug 13, 2017
Merged

Add Snappy package #949

merged 1 commit into from
Aug 13, 2017

Conversation

chfast
Copy link
Contributor

@chfast chfast commented Aug 9, 2017

No description provided.

@chfast
Copy link
Contributor Author

chfast commented Aug 9, 2017

Can you explain the TOOLCHAIN=analyse failures? The log is hard to parse.

@ruslo
Copy link
Owner

ruslo commented Aug 10, 2017

analyzer thinks that nullpointer can be used in memcpy:

/home/travis/build/ingenue/hunter/_testing/Hunter/_Base/13efc7d/487d0d9/9e4de4b/Build/Snappy/Source/snappy.cc:1376:5: warning: Null pointer passed as an argument to a 'nonnull' parameter
    memcpy(op_ptr_, ip, avail);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~

@chfast
Copy link
Contributor Author

chfast commented Aug 10, 2017

I've seen this, but it is a warning. I'll report upstream.

@ruslo
Copy link
Owner

ruslo commented Aug 10, 2017

I've seen this, but it is a warning

This is how clang analyzer works - it reports warnings. Polly toolchain analyze use script to turn it into error, otherwise this toolchain is useless :)

I'll report upstream.

Okay, but if you're not planning to fix it fast it make sense to exclude it. We can always enable it back if needed.

@chfast
Copy link
Contributor Author

chfast commented Aug 10, 2017

I'd like to disable it.

The issue with memcpy() is known problem. Standard says you cannot pass null pointer there, but everyone knows implementations handles nulls anyway.

@chfast
Copy link
Contributor Author

chfast commented Aug 10, 2017

Can we proceed?

@ruslo
Copy link
Owner

ruslo commented Aug 10, 2017

Feel free to send pull request with exclude to pkg.snappy.

@chfast
Copy link
Contributor Author

chfast commented Aug 11, 2017

Have you tested it?

@ruslo
Copy link
Owner

ruslo commented Aug 11, 2017

Have you tested it?

I haven't received notification that you've updated ingenue#110. Will run tests now.

@ruslo ruslo merged commit f12f62e into ruslo:master Aug 13, 2017
@ruslo
Copy link
Owner

ruslo commented Aug 13, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants