[PATCH v1] Added string conversion utility functions

[Imran Zaman]

Thanks Bill for your comments. Plz see my comments inline.

On Tue, Nov 25, 2014 at 9:26 PM, Bill Spitzak <spitzak at gmail.com> wrote:
>
>
> On 11/24/2014 11:12 PM, Imran Zaman wrote:
>>
>> On Tue, Nov 25, 2014 at 1:15 AM, Bill Spitzak <spitzak at gmail.com> wrote:
>>>
>>>
>>>
>>> On 11/24/2014 11:32 AM, Imran Zaman wrote:
>>>
>>>> [IZ2] This is the case where endptr is used in patch. I can remove
>>>> endptr but it seems its used so i have to keep the existing
>>>> functionality in place.
>>>> + if (!weston_strtoi(pid, &end, 0, &other) || end != pid + 10)
>>>
>>>
>>>
>>> Use strlen(pid) != 10 instead.
>>
>>
>> This will not exactly replace the implemented logic e.g.
>> pid="123abcdefg".
>> strlen(pid) = 10
>> end = 3 (end != pid + 10)
>> So cant use the solution which u propose...
>
>
> But weston_strtoi will return false in that case.
pid="123456789".
strlen(pid) = 10
end = pid + 10
.. endptr is needed by the caller of the function..

>
>>> If I write the following code:
>>>
>>>    if (strtol(string, 0, 0, 0)) ...
>>>
>>> I want it to crash so I notice that I passed 0 for the val output
>>> pointer.
>>> There is no reason to every pass null here so it must be a programmer
>>> mistake. As you have written it this will act like there is a syntax
>>> error
>>> in string, which could lead a person trying to debug their code down the
>>> wrong path.
>>>
>> Sorry, why is it a programmer mistake and how would it crash? Let me
>> add the exact test code and will come back to you. It should not crash
>> for sure IMO.
>
>
> struct Foo {
>   int first_member;
>   int integer;
> } Foo;
>
> struct Foo* f(char* text) {
>   struct Foo* pointer;
>   pointer = code_that_returns_NULL_unexpectedly();
>   weston_strtoi(text, 0, 0, &(p->integer)); // crash!
>   return pointer;
> }
Crash happens before entering the function :-) The whole weston code
base uses pointers, so cant be avoided; its user of the functions
which should pass in the valid arguments. So there is no need to worry
about it :-)

>
> Note that with your version it does not crash if "integer" is the first
> member of struct Foo. What I recommend is making it crash in both cases by
> not testing the val pointer and instead trying to write a value to it.
>
>> At least one case above is using endptr.. if we can replace it with
>> exactly similarly logic, I can remove it otherwies I dont see any harm
>> in its usage
>
>
> I have looked into that a bit more. The only use of endptr (other than to
> replicate the tests that your function does) is in the xpm parser. In that
> case I am not sure if it may be depending on getting a zero for zero-length.
> It may be best to leave that code calling the original function rather than
> risk breaking the parser.
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel