On Thu, 7 Dec 2023, Reinette Chatre wrote:
On 12/7/2023 6:32 AM, Ilpo Järvinen wrote:
On Tue, 28 Nov 2023, Reinette Chatre wrote:
On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
...
- /*
* Measure llc occupancy from resctrl.*/- if (!strncmp(param->resctrl_val, CMT_STR, sizeof(CMT_STR))) {
ret = get_llc_occu_resctrl(&llc_occu_resc);if (ret < 0)return ret;llc_value = llc_occu_resc;- }
- ret = print_results_cache(param->filename, bm_pid, llc_value);
- if (ret)
- ret = print_results_cache(filename, bm_pid, llc_perf_miss);
- return ret;
+}
Perhaps print_results_cache() can be made to return negative error and this just be "return print_results_cache(...)" and the function comment be accurate?
I think, I'll just change all "return errno;" to "return -1" before this, however, one open question which impacts whether this is actually Fixes class issue:
It seems that perror()'s manpage doesn't answer one important question, whether it ifself can alter errno or not. The resctrl selftest code assumes it doesn't but some evidence I came across says otherwise so doing return errno; after calling perror() might not even be valid at all.
So I'm tempted to create an additional Fixes patch about the return change into the front of the series.
I would not trust errno to contain code of earlier calls after a call to perror(). If errno is needed I think it should be saved before calling perror(). Looking at perror() at [1] I do not see an effort to restore errno before it returns, and looking at the implementation of perror() there appears to be many opportunities for errno to change.
Reinette
[1] https://sourceware.org/git/?p=glibc.git%3Ba=blob%3Bf=stdio-common/perror.c%3...
I already spent some moments in converting all return error -> return -1, since all such places do perror() calls anyway (which I also converted to ksft_perror() or ksft_print_msg() where perror() didn't make any sense) there's not much added value in returning the errno which was not correctly done in the existing code anyway.