Skip to content

Conversation

@jukkar
Copy link
Member

@jukkar jukkar commented Aug 5, 2019

This is related to findings in #17997 and changes network
related header files to have include files outside of
extern "C" { } block.

Signed-off-by: Jukka Rissanen [email protected]

Copy link
Contributor

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Looks good assuming there are no additional #includes later in a header (I haven't checked).

Thanks for stepping up.

@pfalcon
Copy link
Contributor

pfalcon commented Aug 5, 2019

This PR doesn't seem to fix any bug (no "Fixes: " header). And neither the commit message not referenced #17997 explain what issue is being fixed either.

@mrfuchs
Copy link
Contributor

mrfuchs commented Aug 5, 2019

This PR doesn't seem to fix any bug (no "Fixes: " header). And neither the commit message not referenced #17997 explain what issue is being fixed either.

@pfalcon It fixes linkage errors when including one of these header files in a C++ module. Before the original PR #17993 (referenced in #17997), this was the exact error message (thrown by GCC):

In file included from ../zephyr/include/sched_priq.h:9,
                 from ../zephyr/include/kernel_includes.h:23,
                 from ../zephyr/include/kernel.h:17,
                 from ../zephyr/include/nvs/nvs.h:15,
                 from ...:
../zephyr/include/misc/util.h:53:1: error: template with C linkage
 template < class T, size_t N >
 ^~~~~~~~
In file included ...:
../zephyr/include/nvs/nvs.h:11:1: note: 'extern "C"' linkage started here
 extern "C" {
 ^~~~~~~~~~

Sorry for not mentioning this in the commit message.

@pfalcon
Copy link
Contributor

pfalcon commented Aug 5, 2019

@mrfuchs: Thanks for explanation. Yes, it would have been good if more details went into the commit message.

This is related to findings in zephyrproject-rtos#17997 and changes network related
header files to have include files outside of extern "C" { } block.

Declarations that use C linkage should be placed within extern "C"
so the language linkage is correct when the header is included by
a C++ compiler.

Similarly #include directives should be outside the extern "C" to
ensure the language-specific default linkage is applied to any
declarations provided by the included header.

See: https://en.cppreference.com/w/cpp/language/language_linkage

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the net-fix-extern-c-usage-bug-17997 branch from e12a0c5 to f341c78 Compare August 6, 2019 07:09
@jukkar
Copy link
Member Author

jukkar commented Aug 6, 2019

Updated the commit message.

Copy link
Contributor

@pfalcon pfalcon left a comment

Choose a reason for hiding this comment

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

Updated the commit message.

Thanks!

@jukkar jukkar merged commit 5c05ef5 into zephyrproject-rtos:master Aug 6, 2019
@jukkar jukkar deleted the net-fix-extern-c-usage-bug-17997 branch August 6, 2019 11:46
@backporting
Copy link

backporting bot commented Aug 6, 2019

The backport to v1.14-branch failed:

Commits ["f341c7862a803457a2d394fd64b2364033aa1fce"] could not be cherry-picked on top of v1.14-branch

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport v1.14-branch
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick f341c7862a803457a2d394fd64b2364033aa1fce
# Create a new branch with these backported commits.
git checkout -b backport-18023-to-v1.14-branch
# Push it to GitHub.
git push --set-upstream origin backport-18023-to-v1.14-branch
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is v1.14-branch and the compare/head branch is backport-18023-to-v1.14-branch.

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

Labels

area: API Changes to public APIs area: Networking

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants