[Mesa-dev] [PATCH v2 0/7] port _mesa_strto[df] to C

[Erik Faye-Lund]

On Fri, Jun 26, 2015 at 6:14 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Fri, Jun 26, 2015 at 12:03 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 25 June 2015 at 23:10, Matt Turner <mattst88 at gmail.com> wrote:
>>> On Thu, Jun 25, 2015 at 2:05 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>>>> Back in March[1], I sent a patch porting _mesa_strto[df] to
>>>> C rather than C++. I fixed up the patch according to the
>>>> criticism, but unfortunately I dropped the ball before I sent
>>>> out the result. So here I am, picking it back up!
>>>>
>>>> This time I've taken a deeper dive into the whole init/deinit
>>>> of Mesa, and cleaned up a bunch of stuff in that area. And as
>>>> a result, this time we end up freeing the locale also.
>>>>
>>>> No Piglit regressions observed.
>>>>
>>>> [1]: <1426446329-23984-1-git-send-email-kusmabite at gmail.com>
>>>>
>>>> Erik Faye-Lund (7):
>>>>   mesa/main: Get rid of outdated GDB-hack
>>>>   dri: don't touch the shader compiler
>>>>   mesa/main: only call _mesa_destroy_shader_compiler once on exit
>>>>   glsl: No need to lock in _mesa_glsl_release_types
>>>>   util: port _mesa_strto[df] to C
>>>>   mesa/main: free locale at exit
>>>>   util: assert to verify that locale is initialized
>>>
>>> Thanks for this! The series looks good to me.
>>>
>>> I'm slightly worried about 2/7, but not for any reasons other than I'm
>>> not very familiar with that code.
>>>
>>> 1-6 (with the caveat that I may not have any idea what I'm saying
>>> about 2/7 ;) are:
>>>
>>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>>>
>>> 7/7 looks like it may have been useful for debugging, but I don't
>>> think we should necessarily commit it.
>> 7/7 was suggested when the conversion was hooked up in mesa alone ;-)
>> So I've hinted that glsl-compiler (and maybe others) are left out and
>> adding an assert will help us catch them. If they are fixed now we can
>> drop the patch.
>>
>> I'm fairly confident that the offline compilers (i965?, nouveau and
>> freedreno) and the i965 tests do not use _mesa_strto{d,f} (either
>> directly or not). I'm believe that you've already checked ?
>
> nouveau (and freedreno) compilers use tgsi_text, which definitely can
> parse floats and doubles. No idea what mechanism it uses for that
> though.

tgsi_text call strtod and strtof directly. This means that it breaks
on some locales.

So that should probably be fixed. But that can happen as a follow-up.