Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove stringstream dependency #1117

Merged
merged 3 commits into from
Jun 1, 2018
Merged

remove stringstream dependency #1117

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64acb42
remove stringstream dependency
TinyTinni May 31, 2018
48656a4
typo
TinyTinni May 31, 2018
ecadcdb
added char cast
TinyTinni May 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions include/nlohmann/detail/input/binary_reader.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@
#include <cmath> // ldexp
#include <cstddef> // size_t
#include <cstdint> // uint8_t, uint16_t, uint32_t, uint64_t
#include <cstdio> // snprinft
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: snprintf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks

#include <cstring> // memcpy
#include <iomanip> // setw, setfill
#include <ios> // hex
#include <iterator> // back_inserter
#include <limits> // numeric_limits
#include <sstream> // stringstream
#include <string> // char_traits, string
#include <utility> // make_pair, move

Expand Down Expand Up @@ -1665,9 +1663,9 @@ class binary_reader
*/
std::string get_token_string() const
{
std::stringstream ss;
ss << std::setw(2) << std::uppercase << std::setfill('0') << std::hex << current;
return ss.str();
char cr[3];
snprintf(cr, 3, "%.2X", current);
return std::string{cr};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better for NRVO purposes if you do it like this (untested, just speculation)

std::string cr{"  "};
snprintf(&cr[0], 3, "%.2X", current);
return cr;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that C++11 made that undefined, and the non-const data() doesn't exist until C++17, so never mind.

Copy link
Contributor Author

@TinyTinni TinyTinni May 31, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with non-const .data() it is undefined.

The pointer is such that the range [data(); data() + size()) is valid

The main problem is the null-termination of snprintf, which always results in a string with an +1 increased stringlength compared to the the string created before.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine, size() is 2, so cr[0] to cr[2] is valid, and that's exactly what snprintf with a size of 3 will write to, including the nul terminator. In any case, we can't use it, as we can only require C++11.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Scrolling down to the return value, the definition interval is closed

data() + i == &operator for every i in [0, size()]. | (since C++11)

while in the brief, the definition interval is only an open one which excludes [size()].

}

private:
Expand Down
11 changes: 4 additions & 7 deletions include/nlohmann/detail/input/lexer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
#include <clocale> // localeconv
#include <cstddef> // size_t
#include <cstdlib> // strtof, strtod, strtold, strtoll, strtoull
#include <cstdio> // snprintf
#include <initializer_list> // initializer_list
#include <ios> // hex, uppercase
#include <iomanip> // setw, setfill
#include <sstream> // stringstream
#include <string> // char_traits, string
#include <vector> // vector

Expand Down Expand Up @@ -1174,10 +1172,9 @@ class lexer
if ('\x00' <= c and c <= '\x1F')
{
// escape control characters
std::stringstream ss;
ss << "<U+" << std::setw(4) << std::uppercase << std::setfill('0')
<< std::hex << static_cast<int>(c) << ">";
result += ss.str();
char cs[9];
snprintf(cs, 9, "<U+%.4X>", c);
result += cs;
}
else
{
Expand Down
15 changes: 6 additions & 9 deletions include/nlohmann/detail/output/serializer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,8 @@
#include <cstddef> // size_t, ptrdiff_t
#include <cstdint> // uint8_t
#include <cstdio> // snprintf
#include <iomanip> // setfill
#include <iterator> // next
#include <limits> // numeric_limits
#include <string> // string
#include <sstream> // stringstream
#include <type_traits> // is_same

#include <nlohmann/detail/exceptions.hpp>
Expand Down Expand Up @@ -389,9 +386,9 @@ class serializer

case UTF8_REJECT: // decode found invalid UTF-8 byte
{
std::stringstream ss;
ss << std::setw(2) << std::uppercase << std::setfill('0') << std::hex << static_cast<int>(byte);
JSON_THROW(type_error::create(316, "invalid UTF-8 byte at index " + std::to_string(i) + ": 0x" + ss.str()));
std::string sn(3, '\0');
snprintf(&sn[0], sn.size(), "%.2X", byte);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about changing this to:
snprintf(&sn[0], sn.size(), "%.2X", static_cast<uint8_t>(s[i]));

In order to avoid "‘%.2X’ directive output may be truncated writing between 2 and 8 bytes into a region of size 3 [-Wformat-truncation=]" (GCC 7.3.0).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the difference.
byte is defined as
const auto byte = static_cast<uint8_t>(s[i]);
So it is 1 byte and the same, what you wrote.
Maybe a false positive in GCC?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose it might be a false positive since there is no difference.
Maybe someone with the same GCC version can confirm this warning.

JSON_THROW(type_error::create(316, "invalid UTF-8 byte at index " + std::to_string(i) + ": 0x" + sn));
}

default: // decode found yet incomplete multi-byte code point
Expand All @@ -417,9 +414,9 @@ class serializer
else
{
// we finish reading, but do not accept: string was incomplete
std::stringstream ss;
ss << std::setw(2) << std::uppercase << std::setfill('0') << std::hex << static_cast<int>(static_cast<uint8_t>(s.back()));
JSON_THROW(type_error::create(316, "incomplete UTF-8 string; last byte: 0x" + ss.str()));
std::string sn(3,'\0');
snprintf(&sn[0], sn.size(), "%.2X", s.back());
JSON_THROW(type_error::create(316, "incomplete UTF-8 string; last byte: 0x" + sn));
}
}

Expand Down