mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rojas <alexan...@mesosphere.io>
Subject Re: Review Request 55410: Used standard classic locale for `jsonify`.
Date Wed, 11 Jan 2017 11:05:55 GMT

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



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).

- Alexander Rojas


On Jan. 11, 2017, 11: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, 11: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