Skip to content

native: fix warning errors freebsd#3390

Merged
thomaseichinger merged 3 commits intoRIOT-OS:masterfrom
thomaseichinger:pr/fix_warning_errors_freebsd
Jul 23, 2015
Merged

native: fix warning errors freebsd#3390
thomaseichinger merged 3 commits intoRIOT-OS:masterfrom
thomaseichinger:pr/fix_warning_errors_freebsd

Conversation

@thomaseichinger
Copy link
Copy Markdown
Member

Fixes warnings on FreeBSD which became errors in #3319.

@thomaseichinger thomaseichinger added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jul 14, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is defined as void *ss_sp; on Linux, this cast may introduce additional warnings about alignment. Any ideas?
Do we need an ifdef?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm, would guess #ifdef it is then.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

normally you can cast void * pointers from and to anything... That's the point of void * pointers: being generic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also keep to the coding conventions: (char *)stk;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After looking through the code some more I think that stk should be changed into a void *

Edit: ... or a char *, whatever works best

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@authmillenon The problem is, on FreeBSD it is not a void * but char * pointer.

@thomaseichinger
Copy link
Copy Markdown
Member Author

updated addressing comments.

@jnohlgard
Copy link
Copy Markdown
Member

@thomaseichinger how about changing the stk variable into a void * or char *?
The whole pointer math part around #ifdef NATIVESPONTOP seem to assume stk is a void **

@thomaseichinger thomaseichinger force-pushed the pr/fix_warning_errors_freebsd branch 2 times, most recently from c220bd2 to d39ab04 Compare July 17, 2015 17:58
@thomaseichinger
Copy link
Copy Markdown
Member Author

@authmillenon could you check these commits tackle the warnings we talked about?

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jul 17, 2015

Yes, they are.

@miri64 miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 18, 2015
@thomaseichinger thomaseichinger force-pushed the pr/fix_warning_errors_freebsd branch from d39ab04 to 2052c2c Compare July 21, 2015 16:15
@thomaseichinger thomaseichinger added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Jul 21, 2015
@thomaseichinger thomaseichinger force-pushed the pr/fix_warning_errors_freebsd branch from 2052c2c to cbc9ddc Compare July 21, 2015 16:36
@thomaseichinger
Copy link
Copy Markdown
Member Author

@gebart I adapted stk and remove NATIVESPONTOP as it seems it is never used. @LudwigOrtmann can you verify this.

@jnohlgard
Copy link
Copy Markdown
Member

I'm assuming that the nativespontop is something that have been an experiment with upward growing stack.

@jnohlgard
Copy link
Copy Markdown
Member

I have some ideas for a refactoring with creating stack arrays as unsigned long instead in order to get rid of some Wextra warnings,but that is outside of the scope of this PR.

@jnohlgard
Copy link
Copy Markdown
Member

ACK, go after Travis is fine and @LudwigOrtmann has had the opportunity to respond to the question by @thomaseichinger

thomaseichinger added a commit that referenced this pull request Jul 23, 2015
…reebsd

native: fix warning errors freebsd
@thomaseichinger thomaseichinger merged commit e416937 into RIOT-OS:master Jul 23, 2015
@thomaseichinger
Copy link
Copy Markdown
Member Author

Got the ok of @LudwigOrtmann on a side channel.

char *thread_stack_init(thread_task_func_t task_func, void *arg, void *stack_start, int stacksize)
{
unsigned int *stk;
char *stk;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This broke native running on arm.

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

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Platform: native Platform: This PR/issue effects the native platform Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants