Thanks Bill for your comments. Plz see my comments inline. On Tue, Nov 25, 2014 at 9:26 PM, Bill Spitzak <spit...@gmail.com> wrote: > > > On 11/24/2014 11:12 PM, Imran Zaman wrote: >> >> On Tue, Nov 25, 2014 at 1:15 AM, Bill Spitzak <spit...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel