[PATCH v1] Added string conversion utility functions

Bill Spitzak spitzak at gmail.com
Tue Nov 25 11:26:56 PST 2014



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.

>> 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;
}

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.



More information about the wayland-devel mailing list