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.

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 the new rcutils_strerror function. #239

Merged
merged 3 commits into from Feb 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion tf2/CMakeLists.txt
Expand Up @@ -10,6 +10,7 @@ find_package(ament_cmake)
find_package(console_bridge_vendor REQUIRED) # Provides console_bridge 0.4.0 on platforms without it.
find_package(console_bridge REQUIRED)
find_package(geometry_msgs REQUIRED)
find_package(rcutils REQUIRED)

include_directories(include src/bt)

Expand All @@ -20,6 +21,7 @@ add_library(tf2 SHARED src/cache.cpp src/buffer_core.cpp src/static_cache.cpp sr
ament_target_dependencies(tf2
"console_bridge"
"geometry_msgs"
"rcutils"
)
# Causes the visibility macros to use dllexport rather than dllimport,
# which is appropriate when building the dll but not consuming it.
Expand Down Expand Up @@ -77,7 +79,7 @@ if(BUILD_TESTING)

endif()

ament_export_dependencies(console_bridge geometry_msgs)
ament_export_dependencies(console_bridge geometry_msgs rcutils)
ament_export_include_directories(include)
ament_export_libraries(tf2)
ament_package()
3 changes: 2 additions & 1 deletion tf2/package.xml
Expand Up @@ -23,7 +23,8 @@
<depend>console_bridge_vendor</depend>
<depend>geometry_msgs</depend>
<depend>libconsole-bridge-dev</depend>

<depend>rcutils</depend>

<test_depend>ament_cmake_gtest</test_depend>
<!-- TODO(tfoote) add linting
<test_depend>ament_lint_auto</test_depend>
Expand Down
36 changes: 11 additions & 25 deletions tf2/src/time.cpp
Expand Up @@ -29,45 +29,31 @@

/** \author Tully Foote */

#include <errno.h>
#include <string.h>
#include <stdio.h>
#include <time.h>
#include <stdexcept>
#include <string>

#include "rcutils/snprintf.h"
#include "rcutils/strerror.h"
#include "tf2/time.h"

using namespace tf2;

std::string tf2::displayTimePoint(const TimePoint& stamp)
std::string tf2::displayTimePoint(const tf2::TimePoint& stamp)
{
const char * format_str = "%.6f";
double current_time = timeToSec(stamp);
double current_time = tf2::timeToSec(stamp);
int buff_size = snprintf(NULL, 0, format_str, current_time);
if (buff_size < 0) {
#ifdef _WIN32
// Using fixed buffer size since, strerrorlen_s not yet available
const int errormsglen = 200;
char errmsg[errormsglen];
strerror_s(errmsg, errormsglen, errno);
char errmsg[200];
rcutils_strerror(errmsg, sizeof(errmsg));
throw std::runtime_error(errmsg);
#else
throw std::runtime_error(strerror(errno));
#endif // _WIN32
}

char * buffer = new char[buff_size];
int bytes_written = snprintf(buffer, buff_size, format_str, current_time);
int bytes_written = rcutils_snprintf(buffer, buff_size, format_str, current_time);
if (bytes_written < 0) {
delete[] buffer;
#ifdef _WIN32
// Using fixed buffer size since, strerrorlen_s not yet available
const int errormsglen = 200;
char errmsg[errormsglen];
strerror_s(errmsg, errormsglen, errno);
char errmsg[200];
rcutils_strerror(errmsg, sizeof(errmsg));
throw std::runtime_error(errmsg);
#else
throw std::runtime_error(strerror(errno));
#endif // _WIN32
Copy link
Member

Choose a reason for hiding this comment

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

Instead, replace all of this with rcutils_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.

For consistencies sake, I did switch to rcutils_snprintf. As far as I can tell, this doesn't remove the need for the rest of the error checking, so I left the rest as-is.

Copy link
Member

Choose a reason for hiding this comment

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

Oh maybe I meant to say format string, it does the whole “how big? Allocate, snprintf, error handling” thing:

https://github.com/ros2/rcutils/blob/bd590e5253672957e8c7e070523fa725a4187db2/src/format_string.c#L32

Though it would use an rcutils allocator instead.

Idiomatic c++ would use streams instead... or the format stuff if we had c++20.

}
std::string result = std::string(buffer);
delete[] buffer;
Expand Down