[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