Skip to content

kw2xrf: use static inline #7108

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.

Sign up for GitHub

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

Jump to bottom
Merged
cgundogan merged 1 commit into from
May 30, 2017
Merged

kw2xrf: use static inline #7108

cgundogan merged 1 commit into from
May 30, 2017

Conversation

@cgundogan
Copy link
Member

@cgundogan cgundogan commented May 30, 2017

fixes #7090

@cgundogan cgundogan added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers labels May 30, 2017
@cgundogan cgundogan requested review from jfischer-no and smlng May 30, 2017 07:52
@cgundogan cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 30, 2017
@jnohlgard
Copy link
Member

jnohlgard commented May 30, 2017

I don't see why the linking failures happened in the first place.

@miri64
Copy link
Member

miri64 commented May 30, 2017

@gebart because multiple c-files include drivers/kw2xrf/include/kw2xrf_intern.h, the compiler decides to not make the function inline (because sizewise it would be more ideal) => function gets global name with first c-file => second c-file tries to do the same => boom.

Maybe it even makes more sense to not make these functions inline. This should however be evaluated with a size check.

@jnohlgard
Copy link
Member

jnohlgard commented May 30, 2017

I see. I would have expected it to become multiple definition rather than undefined reference.

@jnohlgard
Copy link
Member

jnohlgard commented May 30, 2017

It's possible this affects my #7107 as well.

smlng
smlng approved these changes May 30, 2017
View reviewed changes
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

ACK, (still) works with gcc 5 and 6

@cgundogan
Copy link
Member Author

cgundogan commented May 30, 2017

murdock agrees - GO

@cgundogan cgundogan merged commit fd7f8a5 into RIOT-OS:master May 30, 2017
@cgundogan cgundogan deleted the pr/kw2xrf_inline branch May 30, 2017 18:21
@aabadie aabadie modified the milestone: Release 2017.07 Jun 30, 2017
@smlng smlng mentioned this pull request Jul 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

1 more reviewer

@smlng smlng smlng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@smlng smlng

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2017.07

Development

Successfully merging this pull request may close these issues.

kw2xrf: linking problems with arm-none-eabi-gcc 7.1.0 and *-ld 2.28

5 participants

@cgundogan @jnohlgard @miri64 @smlng @aabadie