src/util/virstring.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstring.h | 3 ++ src/util/virutil.c | 63 ---------------------------------------- src/util/virutil.h | 3 -- 4 files changed, 84 insertions(+), 66 deletions(-)
The commits add locale support for virStrToDouble() due to differences between the mantissa separator in different languages. For example, kernel always uses dot to separate mantissa. An user who is using pt_BR locale (for example) uses comma as a separator. So, this user will have problems to parse a kernel settings using strtod() function. One of commits move the virDoubleToStr() to virstring.* to share locale global variables. Joining the two functions makes more sense. Julio Faracco (2): util: moving virDoubleToStr() from virutil to virstring. util: fix locale problem with virStrToDouble(). src/util/virstring.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstring.h | 3 ++ src/util/virutil.c | 63 ---------------------------------------- src/util/virutil.h | 3 -- 4 files changed, 84 insertions(+), 66 deletions(-) -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote: >The commits add locale support for virStrToDouble() due to differences between >the mantissa separator in different languages. For example, kernel always uses >dot to separate mantissa. An user who is using pt_BR locale (for example) uses >comma as a separator. So, this user will have problems to parse a kernel >settings using strtod() function. > >One of commits move the virDoubleToStr() to virstring.* to share locale >global variables. Joining the two functions makes more sense. > Reviewed-by: Martin Kletzander <mkletzan@redhat.com> I'll push it in a minute. Thanks for the patches and patience! >Julio Faracco (2): > util: moving virDoubleToStr() from virutil to virstring. > util: fix locale problem with virStrToDouble(). > > src/util/virstring.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/util/virstring.h | 3 ++ > src/util/virutil.c | 63 ---------------------------------------- > src/util/virutil.h | 3 -- > 4 files changed, 84 insertions(+), 66 deletions(-) > >-- >2.7.4 > >-- >libvir-list mailing list >libvir-list@redhat.com >https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 22, 2017 at 11:30:06 +0200, Martin Kletzander wrote: > On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote: > > The commits add locale support for virStrToDouble() due to differences between > > the mantissa separator in different languages. For example, kernel always uses > > dot to separate mantissa. An user who is using pt_BR locale (for example) uses > > comma as a separator. So, this user will have problems to parse a kernel > > settings using strtod() function. > > > > One of commits move the virDoubleToStr() to virstring.* to share locale > > global variables. Joining the two functions makes more sense. > > > > Reviewed-by: Martin Kletzander <mkletzan@redhat.com> > > I'll push it in a minute. Thanks for the patches and patience! Since this broke build and will require fixing. I'd prefer that the code to set and revert the locale will be wrapped into a function rather than scattering conditionally compiled code through the code base. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 22, 2017 at 01:12:44PM +0200, Peter Krempa wrote: >On Thu, Jun 22, 2017 at 11:30:06 +0200, Martin Kletzander wrote: >> On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote: >> > The commits add locale support for virStrToDouble() due to differences between >> > the mantissa separator in different languages. For example, kernel always uses >> > dot to separate mantissa. An user who is using pt_BR locale (for example) uses >> > comma as a separator. So, this user will have problems to parse a kernel >> > settings using strtod() function. >> > >> > One of commits move the virDoubleToStr() to virstring.* to share locale >> > global variables. Joining the two functions makes more sense. >> > >> >> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> >> >> I'll push it in a minute. Thanks for the patches and patience! > >Since this broke build and will require fixing. I'd prefer that the >code to set and revert the locale will be wrapped into a function rather >than scattering conditionally compiled code through the code base. I'm working on that, but either we need more functions to add conditionally, or just remove the conditionally compiled code from just one of those two functions. I'll post a fix in a while that fixes and cleans up more stuff, so we'll see and can talk on that patch. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Twos question related to this topic:
1. Is virstringtest missing some important tests from virstring.c? I'm
not seeing vir{StrToDouble|DoubleToStr}...
2. Should locale be considered during the test phase too?
2017-06-22 8:23 GMT-03:00 Martin Kletzander <mkletzan@redhat.com>:
> On Thu, Jun 22, 2017 at 01:12:44PM +0200, Peter Krempa wrote:
>>
>> On Thu, Jun 22, 2017 at 11:30:06 +0200, Martin Kletzander wrote:
>>>
>>> On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote:
>>> > The commits add locale support for virStrToDouble() due to differences
>>> > between
>>> > the mantissa separator in different languages. For example, kernel
>>> > always uses
>>> > dot to separate mantissa. An user who is using pt_BR locale (for
>>> > example) uses
>>> > comma as a separator. So, this user will have problems to parse a
>>> > kernel
>>> > settings using strtod() function.
>>> >
>>> > One of commits move the virDoubleToStr() to virstring.* to share locale
>>> > global variables. Joining the two functions makes more sense.
>>> >
>>>
>>> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
>>>
>>> I'll push it in a minute. Thanks for the patches and patience!
>>
>>
>> Since this broke build and will require fixing. I'd prefer that the
>> code to set and revert the locale will be wrapped into a function rather
>> than scattering conditionally compiled code through the code base.
>
>
> I'm working on that, but either we need more functions to add
> conditionally, or just remove the conditionally compiled code from just
> one of those two functions. I'll post a fix in a while that fixes and
> cleans up more stuff, so we'll see and can talk on that patch.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 22, 2017 at 05:55:12PM -0300, Julio Faracco wrote:
>Twos question related to this topic:
>1. Is virstringtest missing some important tests from virstring.c? I'm
>not seeing vir{StrToDouble|DoubleToStr}...
>2. Should locale be considered during the test phase too?
>
Yep, it would be nice to have a test for that. We can certainly do
that, it's just that nobody did that yet. Patches are welcome, though ;)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.