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;a=blob;f=stdio-common/perror.c;h=51e621e332a5e2aa76ecefb3bcf325efb43b345f;hb=HEAD#l47
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.
--
i.
