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

Use stringstream instead of stod to work around locale issues. #42

Merged
merged 5 commits into from
Feb 21, 2018
Merged

Use stringstream instead of stod to work around locale issues. #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
739624e
Use stringstream instead of stod to work around locale issues.
clalancette Nov 20, 2017
678ab93
Use imbue to always use the "classic" locale.
clalancette Nov 20, 2017
c5f6b9f
Make a strToDouble facility in the utils.h header.
clalancette Nov 20, 2017
caf2e8c
Add some additional includes.
clalancette Feb 2, 2018
567ba15
Add in an actual error message to the strToDouble throw.
clalancette Feb 2, 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
8 changes: 2 additions & 6 deletions urdf_model/include/urdf_model/pose.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,10 @@ class Vector3
for (unsigned int i = 0; i < pieces.size(); ++i){
if (pieces[i] != ""){
try {
xyz.push_back(std::stod(pieces[i]));
}
catch (std::invalid_argument &/*e*/) {
xyz.push_back(strToDouble(pieces[i].c_str()));
} catch(std::runtime_error &) {
throw ParseError("Unable to parse component [" + pieces[i] + "] to a double (while parsing a vector value)");
}
catch (std::out_of_range &/*e*/) {
throw ParseError("Unable to parse component [" + pieces[i] + "] to a double, out of range (while parsing a vector value)");
}
}
}

Expand Down
25 changes: 25 additions & 0 deletions urdf_model/include/urdf_model/utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
#ifndef URDF_INTERFACE_UTILS_H
#define URDF_INTERFACE_UTILS_H

#include <locale>
#include <sstream>
#include <stdexcept>
#include <string>
#include <vector>

Expand All @@ -62,6 +65,28 @@ void split_string(std::vector<std::string> &result,
}
}

// This is a locale-safe version of string-to-double, which is suprisingly
// difficult to do correctly. This function ensures that the C locale is used
// for parsing, as that matches up with what the XSD for double specifies.
// On success, the double is returned; on failure, a std::runtime_error is
// thrown.
static inline double strToDouble(const char *in)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use const char * instead of const std::string &? Just curious, not asking you to change it, just hoping to learn something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a good (but sneaky) reason for it. I'm hoping that once we get this basic support in, we can use this more generally throughout the urdfdom libraries to avoid this problem. As one example, https://github.com/ros/urdfdom/blob/master/urdf_parser/src/link.cpp#L120 probably suffers from a similar problem (as do several other std::stod conversions in there). Several of those get a const char * from tinyxml, so I figured this interface was more generic rather than having to wrap those inside strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, right, I forgot I also did another PR to actually fix all of that as well: ros/urdfdom#105

{
std::stringstream ss;
Copy link
Contributor

Choose a reason for hiding this comment

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

#include <sstream>?

ss.imbue(std::locale::classic());
Copy link
Contributor

Choose a reason for hiding this comment

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

#include <locale>?


ss << in;

double out;
ss >> out;

if (ss.fail() || !ss.eof()) {
throw std::runtime_error("Failed converting string to double");
}

return out;
}

}

#endif