On Tue, 19 Mar 2024 at 18:21, Andreas Schwab <schwab@suse.de> wrote:

> On Mär 18 2024, Job Snijders wrote:
>
> > On Mon, Mar 18, 2024 at 10:23:27AM +0100, Andreas Schwab wrote:
> >> On Mär 18 2024, Job Snijders wrote:
> >> > +  __set_errno (0);
> >> > +  lbits = strtol(sep, &ep, 10);
> >> > +  if (sep[0] == '\0' || *ep != '\0') {
> >> > +          __set_errno (ENOENT);
> >> > +          return (-1);
> >> > +  }
> >> > +  if ((errno == ERANGE && (lbits == LONG_MAX || lbits == LONG_MIN))
> >> > +      || (lbits > 128 || lbits < 0)) {
> >> > +          __set_errno (EMSGSIZE);
> >> > +          return (-1);
> >>
> >> I think the first half of the error check is redundant since we only
> >> accept values in the range [0,128] anyway.
> >
> > This is an idiomatic error check. The compiler can optimize parts of
> > it, if the compiler feels they are not not neccessary.
>
> But it unnecessarily modifies errno (which the compiler cannot optimize
> out), and sets it to zero when no error occurs, which no library routine
> must ever do.



The proposal at hand does save errno at the start and restores it at the
end to avoid unnecessary stomping on errno (if nothing bad happened). I’m
not entirely sure how removing the idiomatic check helps in that regard? I
might be missing something.

Kind regards,

Job

>