mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Park <mp...@apache.org>
Subject Re: Review Request 55410: Used standard classic locale for `jsonify`.
Date Wed, 11 Jan 2017 18:21:16 GMT


> On Jan. 11, 2017, 3:05 a.m., Alexander Rojas wrote:
> > This patch doesn't really fix the issue, the following program fails to produce
appropriate JSON:
> > 
> > ```c++
> > #include <stout/json.hpp>
> > #include <stout/jsonify.hpp>
> > 
> > #include <clocale>
> > #include <iostream>
> > #include <locale>
> > 
> > int main(int argc, char **argv) {
> >   // Set locale to German.
> >   std::setlocale(LC_ALL, "de_DE.UTF-8");
> >   std::cout.imbue(std::locale("de_DE.UTF-8"));
> > 
> >   std::vector<double> doubles = {1234567890.12345, 123456789.012345};
> >   std::cout << jsonify(doubles) << '\n';
> > 
> >   std::string str = jsonify(doubles);
> >   std::cout << str << '\n';
> >   return 0;
> > }
> > ```
> > 
> > The reason is that the patch changes the locale of the `stream` but printing is
done with `snprintf()`. At the same time, changing the locale of the stream is not thread
safe (although using the stream concurrently should be heavily discouraged).

Synced with Alexander on Slack, we're going to implement `uselocale` on Windows, similar to
how libc++ does here: https://github.com/llvm-mirror/libcxx/blob/1b34b986bcc7e0991513213c295f3c9c82072a34/src/support/win32/locale_win32.cpp#L26-L39


- Michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55410/#review161209
-----------------------------------------------------------


On Jan. 11, 2017, 2:03 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55410/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `<xlocale.h>` header doesn't exist in Windows and thus broke
> the Windows build.
> 
> ```
> fatal error C1083: Cannot open include file: 'xlocale.h': No such file
> ```
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/jsonify.hpp 3c48046e087de2a66139a31449327fd94c149371 
> 
> Diff: https://reviews.apache.org/r/55410/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message