-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8333871: check sysinfo return values #28317
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
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back mbaesken! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
| if (ret == 0) { | ||
| sprops.cpu_isalist = strdup(list); | ||
| } else { | ||
| sprops.cpu_isalist = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind checking the code that puts this in a system property can tolerate NULL? I don't suppose sysinfo will fail here (unless you found a case?) but it would be good to confirm that it won't blow up somewhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do PUTPROP here
| PUTPROP(propArray, _sun_cpu_isalist_NDX, sprops->cpu_isalist); |
with 3rd parameter == NULL in this case.
PUTPROP checks for this so I do not see issues with this
| #define PUTPROP(array, prop_index, val) \ |
In the review of JDK-8333522 it has been discussed that we might run into issues because of failing sysinfo.
However we miss checking the return values at some places, this might need adjustment.
Progress
Warning
8333871: check sysinfo return valuesIssue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28317/head:pull/28317$ git checkout pull/28317Update a local copy of the PR:
$ git checkout pull/28317$ git pull https://git.openjdk.org/jdk.git pull/28317/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28317View PR using the GUI difftool:
$ git pr show -t 28317Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28317.diff
Using Webrev
Link to Webrev Comment