Empy or false string conversion (was :fix string to integer conversion)

Ralph Böhme slow at samba.org
Mon Mar 4 20:00:19 UTC 2019



> Am 01.03.2019 um 08:32 schrieb swen <swen at linux.ibm.com>:
> 
> Hi Ralph
> 
> First of all I changed the subject not that others think this patchset
> is still ongoing.
> 
> On Thu, 2019-02-28 at 13:24 +0100, Ralph Böhme wrote:
>> On Thu, Feb 28, 2019 at 01:16:00PM +0100, swen wrote:
>>> Just out of interest, what needs to be done from your point of view
>>> to
>>> get the last one in as well ?
>> 
>> all callers must be updated by removing all checks but the string not
>> consumed 
>> completey checks. The latter is left to the callers that really need
>> this.
> I'm sorry but I do not get what you're saying.
> 
> I think your initial comment that a no-conserion check is missing,
> is totally valid.
> But I do not get what needs to be changed on the caller side.
> A "no-conversion" situation must be flag'ed by an error to a callee
> (ENODATA might be good) as otherwise the code continues taking a zero
> as a successful ret-val.
> I did not see any code fragment which used this circumstand as 
> "nice" or wanted side-effect.
> 
> However, I did see a few code paths explicitly checking for a full
> string consumption, but those paths were treated as if an error
> occured.
> 
> So could you please explain what is missing where under which
> circumstances ?

eg

ctdb_sock_addr_from_string() currently does

        port = strtoul_err(p+1, &endp, 10, &ret);
        if (endp == p+1 || *endp != '\0' || ret != 0) {
                /* Empty string or trailing garbage */
                return EINVAL;
        }

With your final nail in the coffin this should become just:

        port = strtoul_err(p+1, &endp, 10, &ret);
        if (ret != 0) {
                /* Empty string or trailing garbage */
                return EINVAL;
        }

Then we're at the point where we can rename the function and use a sane calling convention. :)

-slow





More information about the samba-technical mailing list