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.

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

bpo-29859: Fix error messages from return codes for pthread_* calls #741

Merged
merged 3 commits into from
Mar 21, 2017
Merged

bpo-29859: Fix error messages from return codes for pthread_* calls #741

merged 3 commits into from
Mar 21, 2017

Conversation

Birne94
Copy link
Contributor

@Birne94 Birne94 commented Mar 20, 2017

Fix error messages from return codes for pthread_* calls

Before (example call to PyThread_release_lock):

pthread_mutex_lock[3]: Undefined error: 0
pthread_cond_signal: Undefined error: 0
pthread_mutex_unlock[3]: Undefined error: 0

After (example call to PyThread_release_lock):

pthread_mutex_lock[3]: Invalid argument
pthread_cond_signal: Invalid argument
pthread_mutex_unlock[3]: Invalid argument

methane
methane reviewed Mar 21, 2017
View reviewed changes
Python/thread_pthread.h Outdated
@@ -143,6 +143,7 @@ typedef struct {
} pthread_lock;

#define CHECK_STATUS(name) if (status != 0) { perror(name); error = 1; }
#define CHECK_STATUS_PTHREAD(name) if (status != 0) { char* strerror_msg = strerror(status); fprintf(stderr, "%s: %s\n", name, strerror_msg); }
Copy link
Member

Choose a reason for hiding this comment

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

temporary variable seems redundant.
fprintf(stderr, "%s: %s\n", name, strerror(status)) is enough, maybe.

You removed error = 1;. Is it intended change?

Please wrap line within 79 columns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, I have updated the PR. Should I squash both commits?

Copy link
Member

Choose a reason for hiding this comment

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

We use "Squash and merge" button always.
So there is no need to create "crean commit log" in pull request branch.

Would you add an entry to Misc/NEWS too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@methane methane added needs backport to 3.6 type-bug An unexpected behavior, bug, or error labels Mar 21, 2017
@methane
Copy link
Member

methane commented Mar 21, 2017

Uh, NEWS cause conflict. Could you merge origin/master?

@Birne94
Copy link
Contributor Author

Birne94 commented Mar 21, 2017 •
edited
Loading

I rebased my branch against upstream/master.

@methane methane merged commit d7fa6b2 into python:master Mar 21, 2017
@methane methane mentioned this pull request Mar 21, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@methane methane methane approved these changes

Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Birne94 @methane @the-knights-who-say-ni