[PATCH resend] replace: Replace BSD strtoll by wrapping strtoll instead of strtoq
Felix Janda
felix.janda at posteo.de
Mon Jun 22 01:51:26 MDT 2015
Volker Lendecke wrote:
> On Sun, Jun 21, 2015 at 12:03:56PM +0200, Felix Janda wrote:
> > When it is detected that strtoll returns EINVAL not only in the case
> > that the base is not supported, HAVE_BSD_STRTOLL is declared and
> > strtoll is replaced. The current replacement code wraps strtoq in
> > order to replace strtoll and errors out when strtoq is missing.
> >
> > In order to remove this possible error path, we can use strtoll instead
> > of strtoq since the code is only used when it is known that strtoll exists.
> >
> > The fixes a compilation problem on linux systems using musl libc, which
> > has a BSD-like strtoll but no strtoq.
>
> Ok. I'm not 100% sure about whether the replacement function is
> right. We might have to test for ((nb==0)&&(errno==EINVAL)) instead
> of just errno==EINVAL, but as this is a change on top of yours, it's
> probably okay.
The only unexpected behavior of the replacement function I see is that
if errno is EINVAL from a previous function, strtoll does not touch
errno and the base is ok, then errno will be reset to 0.
To fix this changing the condition to (nb==0)&&(errno==EINVAL) does not
seem enough, 0 might be a valid return value of strtoll. Maybe do
something like:
/* glibc's strtoll() sets errno to EINVAL only if base is not ok */
long long int rep_strtoll(const char *str, char **endptr, int base)
{
long long int nb;
int errnoo = errno;
/* Return early if base is not ok */
if (base < 0 || base > 36) {
errno = EINVAL;
return 0;
}
nb = strtoll(str, endptr, base);
if (errno == EINVAL) errno = errnoo;
return nb;
}
Anyway as you said this is for a different commit.
> Reviewed-by: Volker Lendecke <vl at samba.org>
Thanks.
Felix
More information about the samba-technical
mailing list