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

accelio: fix i686-linux build #9570

Merged
merged 2 commits into from
Sep 4, 2015
Merged

accelio: fix i686-linux build #9570

merged 2 commits into from
Sep 4, 2015

Conversation

dfoxfranke
Copy link
Contributor

cc @wkennington

@wkennington
Copy link
Contributor

Is this patch not included upstream? I'm building accelio fine on gcc5 so
I'm not sure why all of this is needed.

On Sun, Aug 30, 2015, 20:55 Daniel Fox Franke notifications@github.com
wrote:

Compile with gcc5 to avoid the compiler bug described in
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg02560.html

Add a patch to fix the many incorrect printf format specifiers and
other sloppy type conversions that gcc5 catches and warns on (erroring out
due to -Werror).

cc @wkennington https://github.com/wkennington

You can view, comment on, or merge this pull request online at:

#9570
Commit Summary

  • accelio: fix i686-linux build

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#9570.

@dfoxfranke
Copy link
Contributor Author

It errors out for me. Are you turning off -Werror or some such?

...and I just noticed it still does on x86_64-linux because there's a conversion I missed, so hold off on merging this until I fix that.

@wkennington
Copy link
Contributor

Nope, but I'm using gcc5 in the stdenv so maybe that changes things?

On Sun, Aug 30, 2015, 21:00 Daniel Fox Franke notifications@github.com
wrote:

It errors out for me. Are you turning off -Werror or some such?

...and I just noticed it still does on x86_64-linux because there's a
conversion I missed, so hold off on merging this until I fix that.


Reply to this email directly or view it on GitHub
#9570 (comment).

@dfoxfranke
Copy link
Contributor Author

Most of those things I changed from %lu to %llu actually need to be changed to PRIu64 (a macro from <inttypes.h>) to be portable. I'll fix it tomorrow.

@wkennington
Copy link
Contributor

You should try and get this upstreamed if not already.

On Sun, Aug 30, 2015, 21:48 Daniel Fox Franke notifications@github.com
wrote:

Most of those things I changed from %lu to %llu actually need to be
changed to PRIu64 (a macro from <inttypes.h>) to be portable. I'll fix it
tomorrow.


Reply to this email directly or view it on GitHub
#9570 (comment).

@dfoxfranke
Copy link
Contributor Author

I will, once I get it right.
On Aug 31, 2015 12:51 AM, "William A. Kennington III" <
notifications@github.com> wrote:

You should try and get this upstreamed if not already.

On Sun, Aug 30, 2015, 21:48 Daniel Fox Franke notifications@github.com
wrote:

Most of those things I changed from %lu to %llu actually need to be
changed to PRIu64 (a macro from <inttypes.h>) to be portable. I'll fix it
tomorrow.


Reply to this email directly or view it on GitHub
#9570 (comment).


Reply to this email directly or view it on GitHub
#9570 (comment).

@lucabrunox
Copy link
Contributor

Beware that %llu may break the x64 version. The correct format for size_t is %zu.

@dfoxfranke
Copy link
Contributor Author

@lethalman The offending arguments are (mostly) uint64_t, not size_t.

@garbas garbas added this to the 15.09 milestone Sep 2, 2015
@garbas garbas added the 2.status: work-in-progress This PR isn't done label Sep 2, 2015
@garbas
Copy link
Member

garbas commented Sep 2, 2015

@dfoxfranke this PR fails for me on x64

@dfoxfranke
Copy link
Contributor Author

@garbas yes, this is currently WIP until I make the %llu -> PRIu64 fix I described above.

@domenkozar
Copy link
Member

cool, let me know when it's ready

@dfoxfranke
Copy link
Contributor Author

I've revised the patch and this should now be ready to go. The patch also fixes all the tests and examples, so in a separate commit I've re-enabled building them and set doCheck = true.

I'll submit these changes upstream once I get back from vacation next week.

@dfoxfranke
Copy link
Contributor Author

Bah... hold off one more moment. I just want to make a cosmetic change to the patch file.

* Compile with gcc5 to avoid the compiler bug described in
  https://gcc.gnu.org/ml/gcc-patches/2014-05/msg02560.html

* Add a patch to fix the many incorrect printf format specifiers and
  other sloppy type conversions that gcc5 catches and warns on
  (erroring out due to -Werror).
The patch committed with 88471b684e6544da7691937a9b68cefa49d260d5
makes them work again.
@dfoxfranke
Copy link
Contributor Author

Done.

domenkozar added a commit that referenced this pull request Sep 4, 2015
accelio: fix i686-linux build
@domenkozar domenkozar merged commit 3ed8c21 into NixOS:master Sep 4, 2015
@domenkozar
Copy link
Member

it breaks kernel module(s) http://hydra.nixos.org/eval/1219660#tabs-now-fail

@dfoxfranke
Copy link
Contributor Author

@domenkozar Revert 8b66350 for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: work-in-progress This PR isn't done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants