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

core: put more intelligence into queue_msg() #1961

Merged
merged 1 commit into from
Nov 7, 2014
Merged

core: put more intelligence into queue_msg() #1961

merged 1 commit into from
Nov 7, 2014

Conversation

Kijewski
Copy link
Contributor

@Kijewski Kijewski commented Nov 7, 2014

Fixes #1942.

There were two instances were it was not checked the target thread has a
message queue before queuing the message.

This PR centralizes the check into queue_msg().

(This PR is part of #1939)

Fixes #1942.

There were two instances were it was not checked the target thread has a
message queue before queuing the message.

This PR centralizes the check into `queue_msg()`.
@Kijewski Kijewski added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: major The PR changes a significant part of the code base. It should be reviewed carefully labels Nov 7, 2014
@Kijewski
Copy link
Contributor Author

Kijewski commented Nov 7, 2014

Tagging as [major], because msg_send_int() was broken.

@kaspar030
Copy link
Contributor

You mention two instances, but fix one?

Still ACK.

@Kijewski
Copy link
Contributor Author

Kijewski commented Nov 7, 2014

This PR fixes msg_send_to_self() and msg_send_int(), but I only think that the latter function is actually important. :)

@miri64
Copy link
Member

miri64 commented Nov 7, 2014

So msg_send_to_self() is now fixed or not?

@miri64
Copy link
Member

miri64 commented Nov 7, 2014

Anyway small progress is better than no progress: ACK

@miri64
Copy link
Member

miri64 commented Nov 7, 2014

NACK

return 0;
DEBUG("queue_msg(): queuing message\n");
msg_t *dest = &target->msg_array[n];
*dest = *m;
Copy link
Member

Choose a reason for hiding this comment

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

Why the double assignment here?

Pointers sometimes still confuse me -.-

@miri64
Copy link
Member

miri64 commented Nov 7, 2014

I revoke my NACK… merge at will.

@Kijewski
Copy link
Contributor Author

Kijewski commented Nov 7, 2014

So msg_send_to_self() is now fixed or not?

Yes, it is.

Kijewski added a commit that referenced this pull request Nov 7, 2014
core: put more intelligence into `queue_msg()`
@Kijewski Kijewski merged commit 7830c86 into RIOT-OS:master Nov 7, 2014
@Kijewski Kijewski deleted the issue-1942 branch November 7, 2014 17:47
@OlegHahm
Copy link
Member

OlegHahm commented Nov 7, 2014

I think the check was already in place by checking the return value of cib_put().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core: msg_send_int() does not check if target has msg_array
4 participants