diff --git a/rosidl_generator_c/rosidl_generator_c/__init__.py b/rosidl_generator_c/rosidl_generator_c/__init__.py index a1454c1d0..bd62c6dde 100644 --- a/rosidl_generator_c/rosidl_generator_c/__init__.py +++ b/rosidl_generator_c/rosidl_generator_c/__init__.py @@ -14,6 +14,8 @@ import re +from rosidl_generator_type_description import parse_rihs_string +from rosidl_generator_type_description import RIHS01_HASH_VALUE_SIZE from rosidl_parser.definition import AbstractGenericString from rosidl_parser.definition import AbstractSequence from rosidl_parser.definition import AbstractString @@ -225,18 +227,14 @@ def escape_wstring(s): def type_hash_to_c_definition(variable_name, hash_string, indent=0): """Generate empy for rosidl_type_hash_t instance with 8 bytes per line for readability.""" - hash_length = 32 bytes_per_line = 8 indent_str = ' ' * (indent + 2) - pattern = re.compile(r'RIHS([0-9a-f]{2})_([0-9a-f]{64})') - match = pattern.match(hash_string) - if not match: - raise Exception('Type hash string does not match expected RIHS format') - version, value = match.group(1, 2) + version, value = parse_rihs_string(hash_string) + assert version == 1, 'This function only knows how to generate RIHS01 definitions.' result = f'rosidl_type_hash_t {variable_name} = {{{version}, {{' - for i in range(hash_length): + for i in range(RIHS01_HASH_VALUE_SIZE): if i % bytes_per_line == 0: result += f'\n{indent_str} ' result += f'0x{value[i * 2:i * 2 + 2]},' diff --git a/rosidl_generator_type_description/resource/HashedTypeDescription.schema.json b/rosidl_generator_type_description/resource/HashedTypeDescription.schema.json index e1b9d05db..1563ff285 100644 --- a/rosidl_generator_type_description/resource/HashedTypeDescription.schema.json +++ b/rosidl_generator_type_description/resource/HashedTypeDescription.schema.json @@ -2,7 +2,7 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "HashedTypeDescription.schema.json", "title": "HashedTypeDescription", - "description": "TODO", + "description": "Contains hashes and full type description for a ROS 2 interface. TypeDescription, IndividualTypeDescription, Field, and FieldType are exact representations of type_description_interfaces/msg types, see their .msg files for semantic comments.", "type": "object", "properties": { "hashes": { @@ -66,11 +66,11 @@ }, "TypeDescription": { "type": "object", - "$comment": "All whitespace must be excluded for consistent hashing, which this schema cannot enforce.", + "$comment": "For hashing: All whitespace must be excluded, which this schema cannot enforce.", "properties": { "type_description": {"$ref": "#/$defs/IndividualTypeDescription"}, "referenced_type_descriptions": { - "$comment": "Referenced type descriptions must be alphabetized, which this schema cannot enforce.", + "$comment": "For hashing: Referenced type descriptions must be alphabetized, which this schema cannot enforce.", "type": "array", "items": { "$ref": "#/$defs/IndividualTypeDescription" } } diff --git a/rosidl_generator_type_description/rosidl_generator_type_description/__init__.py b/rosidl_generator_type_description/rosidl_generator_type_description/__init__.py index 73aeea0ea..ef7e0da10 100644 --- a/rosidl_generator_type_description/rosidl_generator_type_description/__init__.py +++ b/rosidl_generator_type_description/rosidl_generator_type_description/__init__.py @@ -15,14 +15,21 @@ import hashlib import json from pathlib import Path +import re import sys from typing import List, Tuple from rosidl_parser import definition from rosidl_parser.parser import parse_idl_file -# ROS Interface Hashing Standard, per REP-2011 -RIHS_VERSION = '01' +# RIHS: ROS Interface Hashing Standard, per REP-2011 +# NOTE: These values and implementations must be updated if +# - type_description_interfaces messsages change, or +# - the hashing algorithm for type descriptions changes +# Both changes require an increment of the RIHS version +RIHS01_PREFIX = 'RIHS01_' +RIHS01_HASH_VALUE_SIZE = 32 +RIHS01_PATTERN = re.compile(r'RIHS([0-9a-f]{2})_([0-9a-f]{64})') def generate_type_hash(generator_arguments_file: str) -> List[str]: @@ -72,6 +79,15 @@ def generate_type_hash(generator_arguments_file: str) -> List[str]: return generated_files +def parse_rihs_string(rihs_str: str) -> Tuple[int, str]: + """Parse RIHS string, return (version, value) tuple.""" + match = RIHS01_PATTERN.match(rihs_str) + if not match: + raise ValueError(f'Type hash string {rihs_str} does not match expected RIHS format.') + version, value = match.group(1, 2) + return (int(version), value) + + # This mapping must match the constants defined in type_description_interfaces/msgs/FieldType.msg # NOTE: Nonexplicit integer types are not defined in FieldType (short, long, long long). # If a ROS IDL uses these, this generator will throw a KeyError. @@ -446,10 +462,9 @@ def _calculate_hash_tree(self) -> dict: separators=(',', ': '), sort_keys=False ) - prefix = f'RIHS{RIHS_VERSION}_' sha = hashlib.sha256() sha.update(hashable_repr.encode('utf-8')) - type_hash = prefix + sha.hexdigest() + type_hash = RIHS01_PREFIX + sha.hexdigest() type_hash_infos = { self.interface_type: type_hash, diff --git a/rosidl_runtime_c/CMakeLists.txt b/rosidl_runtime_c/CMakeLists.txt index 202a76f7e..5da904f1c 100644 --- a/rosidl_runtime_c/CMakeLists.txt +++ b/rosidl_runtime_c/CMakeLists.txt @@ -30,7 +30,6 @@ target_include_directories(${PROJECT_NAME} PUBLIC ament_target_dependencies(${PROJECT_NAME} "rcutils" "rosidl_typesupport_interface") -target_link_libraries(${PROJECT_NAME} m) if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_C_COMPILER_ID MATCHES "Clang") set_target_properties(${PROJECT_NAME} PROPERTIES COMPILE_OPTIONS -Wall -Wextra -Wpedantic) diff --git a/rosidl_runtime_c/include/rosidl_runtime_c/type_hash.h b/rosidl_runtime_c/include/rosidl_runtime_c/type_hash.h index 7fa66893a..dfe4c516e 100644 --- a/rosidl_runtime_c/include/rosidl_runtime_c/type_hash.h +++ b/rosidl_runtime_c/include/rosidl_runtime_c/type_hash.h @@ -68,6 +68,8 @@ rosidl_stringify_type_hash( /** * \param[in] type_hash_string Null-terminated string with the hash representation * \param[out] hash_out Preallocated structure to be filled with parsed hash information. + * hash_out->version will be 0 if no version could be parsed, + * but if a version could be determined this field will be set even if an error is returned * \return RCTUILS_RET_INVALID_ARGUMENT on any null pointer argumunts, or malformed hash string. * \return RCUTILS_RET_OK otherwise */ diff --git a/rosidl_runtime_c/src/type_hash.c b/rosidl_runtime_c/src/type_hash.c index 46cba9fbf..ba60474ce 100644 --- a/rosidl_runtime_c/src/type_hash.c +++ b/rosidl_runtime_c/src/type_hash.c @@ -12,12 +12,51 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include +#include #include "rosidl_runtime_c/type_hash.h" #include "rcutils/error_handling.h" -#include "rcutils/format_string.h" + +static const char RIHS01_PREFIX[] = "RIHS01_"; +// Hash representation is hex string, two characters per byte +static const size_t RIHS_VERSION_IDX = 4; +static const size_t RIHS_PREFIX_LEN = 7; +static const size_t RIHS01_STRING_LEN = RIHS_PREFIX_LEN + (ROSIDL_TYPE_HASH_SIZE * 2); + +static bool _ishexdigit(char c) +{ + return (c >= '0' && c <= '9') || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f'); +} + +/// Translate a single character hex digit to a nibble +static uint8_t _xatoi(char c) +{ + if (c >= '0' && c <= '9') { + return c - '0'; + } + if (c >= 'A' && c <= 'F') { + return c - 'A' + 0xa; + } + if (c >= 'a' && c <= 'f') { + return c - 'a' + 0xa; + } + return -1; +} + +/// Tranlate a byte value to two hex characters +static void _xitoa(uint8_t val, char * dest) +{ + uint8_t nibble = 0; + for (size_t n = 0; n < 2; n++) { + nibble = (val >> (4 * n)) & 0x0f; + if (nibble < 0xa) { + dest[n] = '0' + nibble; + } else { // 0xa <= nibble < 0x10 + dest[n] = 'a' + (nibble - 0xa); + } + } +} rosidl_type_hash_t rosidl_get_zero_initialized_type_hash(void) @@ -32,55 +71,29 @@ rosidl_stringify_type_hash( rcutils_allocator_t allocator, char ** output_string) { - if (!type_hash) { - RCUTILS_SET_ERROR_MSG("Null type_hash argument."); - return RCUTILS_RET_INVALID_ARGUMENT; - } + RCUTILS_CHECK_ARGUMENT_FOR_NULL(type_hash, RCUTILS_RET_INVALID_ARGUMENT); if (!rcutils_allocator_is_valid(&allocator)) { RCUTILS_SET_ERROR_MSG("Invalid allocator"); return RCUTILS_RET_INVALID_ARGUMENT; } - if (!output_string) { - RCUTILS_SET_ERROR_MSG("Null string destination pointer."); - return RCUTILS_RET_INVALID_ARGUMENT; - } + RCUTILS_CHECK_ARGUMENT_FOR_NULL(output_string, RCUTILS_RET_INVALID_ARGUMENT); - // Hash representation is hex string, two characters per byte - const char * fmt = "RIHS%02d_%64d"; - const size_t prefix_len = strlen("RIHS01_"); - char * local_output = rcutils_format_string(allocator, fmt, type_hash->version, 0); + char * local_output = allocator.allocate(RIHS01_STRING_LEN + 1, allocator.state); if (!local_output) { *output_string = NULL; - RCUTILS_SET_ERROR_MSG("Unable to allocate space for stringified type hash."); + RCUTILS_SET_ERROR_MSG("Unable to allocate space for type hash string."); return RCUTILS_RET_BAD_ALLOC; } + local_output[RIHS01_STRING_LEN] = '\0'; + memcpy(local_output, RIHS01_PREFIX, RIHS_PREFIX_LEN); for (size_t i = 0; i < ROSIDL_TYPE_HASH_SIZE; i++) { - snprintf(local_output + prefix_len + (i * 2), 3, "%02x", type_hash->value[i]); // NOLINT + _xitoa(type_hash->value[i], local_output + RIHS_PREFIX_LEN + (i * 2)); } *output_string = local_output; return RCUTILS_RET_OK; } -static int _xatoi(char c) -{ - if (c >= '0' && c <= '9') { - return c - '0'; - } - if (c >= 'A' && c <= 'F') { - return c - 'A' + 0xa; - } - if (c >= 'a' && c <= 'f') { - return c - 'a' + 0xa; - } - return -1; -} - -static int _str_to_byte(const char * str) -{ - return (_xatoi(str[0]) << 4) + _xatoi(str[1]); -} - rcutils_ret_t rosidl_parse_type_hash_string( const char * type_hash_string, @@ -88,22 +101,46 @@ rosidl_parse_type_hash_string( { RCUTILS_CHECK_ARGUMENT_FOR_NULL(type_hash_string, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ARGUMENT_FOR_NULL(hash_out, RCUTILS_RET_INVALID_ARGUMENT); - static const size_t kprefix_len = sizeof("RIHS01_") - 1; - static const size_t kvalue_len = 64; hash_out->version = 0; + size_t input_len = strlen(type_hash_string); - if (strlen(type_hash_string) != (kprefix_len + kvalue_len)) { - RCUTILS_SET_ERROR_MSG("Hash string incorrect size."); + // Check prefix + if (input_len < RIHS_PREFIX_LEN) { + RCUTILS_SET_ERROR_MSG("Hash string not long enough to contain RIHS prefix."); return RCUTILS_RET_INVALID_ARGUMENT; } - if (0 != strncmp(type_hash_string, "RIHS01_", 7)) { - RCUTILS_SET_ERROR_MSG("Type hash string is not prefixed RIHS01_"); + if (0 != strncmp(type_hash_string, RIHS01_PREFIX, RIHS_VERSION_IDX)) { + RCUTILS_SET_ERROR_MSG("Hash string doesn't start with RIHS."); return RCUTILS_RET_INVALID_ARGUMENT; } - hash_out->version = 1; - const char * value_str = type_hash_string + kprefix_len; - for (size_t i = 0; i < 32; i++) { - int byte_val = _str_to_byte(value_str + (i * 2)); + + // Parse version + char version_top = type_hash_string[RIHS_VERSION_IDX]; + char version_bot = type_hash_string[RIHS_VERSION_IDX + 1]; + if (!(_ishexdigit(version_top) && _ishexdigit(version_bot))) { + RCUTILS_SET_ERROR_MSG("RIHS version is not a 2-digit hex string."); + return RCUTILS_RET_INVALID_ARGUMENT; + } + hash_out->version = (_xatoi(version_top) << 4) + _xatoi(version_bot); + + if (hash_out->version != 1) { + RCUTILS_SET_ERROR_MSG("Do not know how to parse RIHS version."); + return RCUTILS_RET_INVALID_ARGUMENT; + } + if (input_len != RIHS01_STRING_LEN) { + RCUTILS_SET_ERROR_MSG("RIHS string is the incorrect size to contain a RIHS01 value."); + return RCUTILS_RET_INVALID_ARGUMENT; + } + const char * value_str = type_hash_string + RIHS_PREFIX_LEN; + for (size_t i = 0; i < ROSIDL_TYPE_HASH_SIZE; i++) { + if (!_ishexdigit(value_str[i * 2])) { + RCUTILS_SET_ERROR_MSG("Type hash string value contained non-hex-digit character."); + return RCUTILS_RET_INVALID_ARGUMENT; + } + } + for (size_t i = 0; i < ROSIDL_TYPE_HASH_SIZE; i++) { + // No error checking on byte values because the whole string was checked in prior loop + uint8_t byte_val = (_xatoi(value_str[i * 2]) << 4) + _xatoi(value_str[i * 2 + 1]); if (byte_val < 0) { RCUTILS_SET_ERROR_MSG("Type hash string value did not contain only hex digits."); return RCUTILS_RET_INVALID_ARGUMENT; diff --git a/rosidl_runtime_c/test/test_type_hash.cpp b/rosidl_runtime_c/test/test_type_hash.cpp index 8d68a2da2..6c785ce9b 100644 --- a/rosidl_runtime_c/test/test_type_hash.cpp +++ b/rosidl_runtime_c/test/test_type_hash.cpp @@ -92,5 +92,6 @@ TEST(type_hash, parse_bad_version) { "RIHS02_00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff"; rosidl_type_hash_t hash = rosidl_get_zero_initialized_type_hash(); EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, rosidl_parse_type_hash_string(test_value.c_str(), &hash)); + EXPECT_EQ(hash.version, 2); rcutils_reset_error(); }