[PATCH] [tldap] check for successful string conversion

[swen]

On Thu, 2019-01-10 at 11:47 +0100, Volker Lendecke via samba-technical
wrote:
> On Thu, Jan 10, 2019 at 11:35:18AM +0100, swen via samba-technical
> wrote:
> > > > Upon successful completion, these functions shall return the
> > > > converted
> > > > value, if any. If no conversion could be performed, 0 shall be
> > > > returned [Option Start]  and errno may be set to [EINVAL].
> > > > [Option
> > > > End] If the correct value is outside the range of representable
> > > > values, {ULONG_MAX} or {ULLONG_MAX} shall be returned and errno
> > > > set
> > > > to
> > > > [ERANGE].
> > > 
> > > Assuming that all implementations actually do the [Option] piece,
> > > we
> > > need to check for result 0 or ULLONG_MAX *together* with errno.
> > > Just
> > > checking for errno is not sufficient.
> > It is sufficient because errno is set to zero before strtoull() is
> > triggered.
> 
> How do you read the "and" conjunction in the documentation? I would
> rather check for the precise documented conditions. According to the
> documentation, a successful conversion of the string "1" could return
> the number 1 and yet set errno to something !=0. Probably a weird
> implementation, but legal according to the spec.
I read it that "both" will be done in case of an error but regardless
of how often I read the documentation I do not see the possibility
where errno is set but there is no error.

Anyhow, for this patch it is irrelevant whether we fail for RANGE's
sake or because the string doesn't contain anything like a number.
The fact that the conversion fails is enough to know.
This is slightly different for the code snippet you referred to below
where we (seemingly) want to distinguish between the 2 error cases.

> 
> > In fact, this reference is wrong.
> > You cannot assume, that if the return value is zero and errno ==
> > EINVAL
> > that your conversion failed. The errno might have been be set to
> > EINVAL
> > from an earlier command and strtoull just "correctly" returned
> > zero.
> > This way you end up with a false positive.
> 
> You're right. Feel free to fix this.

Ok, will do.

Cheers Swen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190110/66341470/signature.sig>