From 2959d42feb576c0e00c28c4e27658b25f6c783e9 Mon Sep 17 00:00:00 2001 From: Ole Troan Date: Thu, 22 Aug 2019 09:02:59 +0200 Subject: [PATCH] api: use string type for strings in memclnt.api Explicitly using string type in API allows for autogenerating tools to print strings instead of hex-dumping byte strings. Type: fix Signed-off-by: Ole Troan Signed-off-by: Klement Sekera Change-Id: I573962d6b34d5d10aab9dc6a5fdf101c9b12a6a6 Signed-off-by: Ole Troan --- src/vat/api_format.c | 2 +- src/vlibmemory/memclnt.api | 10 +-- src/vlibmemory/memory_api.c | 2 +- src/vlibmemory/memory_client.c | 9 +-- src/vlibmemory/socket_api.c | 3 +- src/vlibmemory/socket_client.c | 4 +- src/vlibmemory/vlib_api.c | 35 ++++++----- src/vpp-api/vapi/vapi_c_gen.py | 5 +- src/vpp-api/vapi/vapi_json_parser.py | 119 ++++++++++++++--------------------- src/vpp/api/custom_dump.c | 8 ++- test/ext/vapi_c_test.c | 13 ++-- test/ext/vapi_cpp_test.cpp | 10 +-- 12 files changed, 104 insertions(+), 116 deletions(-) diff --git a/src/vat/api_format.c b/src/vat/api_format.c index 7efdadc103b..abce80550df 100644 --- a/src/vat/api_format.c +++ b/src/vat/api_format.c @@ -14990,7 +14990,7 @@ api_get_first_msg_id (vat_main_t * vam) } M (GET_FIRST_MSG_ID, mp); - clib_memcpy (mp->name, name, vec_len (name)); + vl_api_vec_to_api_string (name, &mp->name); S (mp); W (ret); return ret; diff --git a/src/vlibmemory/memclnt.api b/src/vlibmemory/memclnt.api index 2f1e2daa4b6..5a7ef4f48ef 100644 --- a/src/vlibmemory/memclnt.api +++ b/src/vlibmemory/memclnt.api @@ -34,7 +34,7 @@ define memclnt_create { u32 context; /* opaque value to be returned in the reply */ i32 ctx_quota; /* requested punt context quota */ u64 input_queue; /* client's queue */ - u8 name[64]; /* for show, find by name, whatever */ + string name [limit=64]; /* for show, find by name, whatever */ u32 api_versions[8]; /* client-server pairs use as desired */ }; @@ -102,7 +102,7 @@ autoreply define rpc_call { define get_first_msg_id { u32 client_index; u32 context; - u8 name[64]; + string name [limit=64]; }; define get_first_msg_id_reply { @@ -118,7 +118,7 @@ typedef module_version { u32 major; u32 minor; u32 patch; - u8 name[64]; + string name [limit=64]; }; define api_versions { u32 client_index; @@ -141,7 +141,7 @@ manual_print define trace_plugin_msg_ids { u32 client_index; u32 context; - u8 plugin_name[128]; + string plugin_name [limit=128]; u16 first_msg_id; u16 last_msg_id; }; @@ -157,7 +157,7 @@ typedef message_table_entry */ define sockclnt_create { u32 context; /* opaque value to be returned in the reply */ - u8 name[64]; /* for show, find by name, whatever */ + string name [limit=64]; /* for show, find by name, whatever */ }; define sockclnt_create_reply { diff --git a/src/vlibmemory/memory_api.c b/src/vlibmemory/memory_api.c index b87aa76b2d0..0dcf0b001af 100644 --- a/src/vlibmemory/memory_api.c +++ b/src/vlibmemory/memory_api.c @@ -211,7 +211,7 @@ vl_api_memclnt_create_t_handler (vl_api_memclnt_create_t * mp) q = regp->vl_input_queue = (svm_queue_t *) (uword) mp->input_queue; - regp->name = format (0, "%s", mp->name); + regp->name = vl_api_from_api_to_vec (&mp->name); vec_add1 (regp->name, 0); if (am->serialized_message_table_in_shmem == 0) diff --git a/src/vlibmemory/memory_client.c b/src/vlibmemory/memory_client.c index f032ae77d19..3e1bdff777c 100644 --- a/src/vlibmemory/memory_client.c +++ b/src/vlibmemory/memory_client.c @@ -195,7 +195,7 @@ vl_client_connect (const char *name, int ctx_quota, int input_queue_size) mp->_vl_msg_id = ntohs (VL_API_MEMCLNT_CREATE); mp->ctx_quota = ctx_quota; mp->input_queue = (uword) vl_input_queue; - strncpy ((char *) mp->name, name, sizeof (mp->name) - 1); + vl_api_to_api_string (strnlen_s (name, 64), name, &mp->name); vl_msg_api_send_shmem (shmem_hdr->vl_input_queue, (u8 *) & mp); @@ -517,7 +517,8 @@ vl_client_get_first_plugin_msg_id (const char *plugin_name) clib_time_t clib_time; u16 rv = ~0; - if (strlen (plugin_name) + 1 > sizeof (mp->name)) + size_t plugin_name_len = strnlen_s (plugin_name, 128); + if (plugin_name_len == 128) return (rv); clib_memset (&clib_time, 0, sizeof (clib_time)); @@ -538,7 +539,7 @@ vl_client_get_first_plugin_msg_id (const char *plugin_name) clib_memset (mp, 0, sizeof (*mp)); mp->_vl_msg_id = ntohs (VL_API_GET_FIRST_MSG_ID); mp->client_index = am->my_client_index; - strncpy ((char *) mp->name, plugin_name, sizeof (mp->name) - 1); + vl_api_to_api_string (plugin_name_len, plugin_name, &mp->name); if (vl_socket_client_write () <= 0) goto sock_err; @@ -563,7 +564,7 @@ vl_client_get_first_plugin_msg_id (const char *plugin_name) clib_memset (mp, 0, sizeof (*mp)); mp->_vl_msg_id = ntohs (VL_API_GET_FIRST_MSG_ID); mp->client_index = am->my_client_index; - strncpy ((char *) mp->name, plugin_name, sizeof (mp->name) - 1); + vl_api_to_api_string (plugin_name_len, plugin_name, &mp->name); vl_msg_api_send_shmem (am->shmem_hdr->vl_input_queue, (u8 *) & mp); diff --git a/src/vlibmemory/socket_api.c b/src/vlibmemory/socket_api.c index 868298ccc85..f9dd53364e4 100644 --- a/src/vlibmemory/socket_api.c +++ b/src/vlibmemory/socket_api.c @@ -436,7 +436,8 @@ vl_api_sockclnt_create_t_handler (vl_api_sockclnt_create_t * mp) ASSERT (regp->registration_type == REGISTRATION_TYPE_SOCKET_SERVER); - regp->name = format (0, "%s%c", mp->name, 0); + regp->name = vl_api_from_api_to_vec (&mp->name); + vec_add1 (regp->name, 0); u32 size = sizeof (*rp) + (nmsg * sizeof (vl_api_message_table_entry_t)); rp = vl_msg_api_alloc_zero (size); diff --git a/src/vlibmemory/socket_client.c b/src/vlibmemory/socket_client.c index 96330ce481a..fcd31992fa5 100644 --- a/src/vlibmemory/socket_client.c +++ b/src/vlibmemory/socket_client.c @@ -395,8 +395,8 @@ vl_socket_client_connect (char *socket_path, char *client_name, mp = vl_socket_client_msg_alloc (sizeof (*mp)); mp->_vl_msg_id = htons (VL_API_SOCKCLNT_CREATE); - strncpy ((char *) mp->name, client_name, sizeof (mp->name) - 1); - mp->name[sizeof (mp->name) - 1] = 0; + + vl_api_to_api_string (strnlen_s (client_name, 64), client_name, &mp->name); mp->context = 0xfeedface; clib_time_init (&scm->clib_time); diff --git a/src/vlibmemory/vlib_api.c b/src/vlibmemory/vlib_api.c index e1a6bd18d55..7d7ed3e68aa 100644 --- a/src/vlibmemory/vlib_api.c +++ b/src/vlibmemory/vlib_api.c @@ -56,10 +56,12 @@ static inline void * vl_api_trace_plugin_msg_ids_t_print (vl_api_trace_plugin_msg_ids_t * a, void *handle) { - vl_print (handle, "vl_api_trace_plugin_msg_ids: %s first %u last %u\n", - a->plugin_name, + u8 *plugin_name = vl_api_from_api_to_vec (&a->plugin_name); + vl_print (handle, "vl_api_trace_plugin_msg_ids: %v first %u last %u\n", + plugin_name, clib_host_to_net_u16 (a->first_msg_id), clib_host_to_net_u16 (a->last_msg_id)); + vec_free (plugin_name); return handle; } @@ -76,7 +78,6 @@ vl_api_get_first_msg_id_t_handler (vl_api_get_first_msg_id_t * mp) uword *p; api_main_t *am = &api_main; vl_api_msg_range_t *rp; - u8 name[64]; u16 first_msg_id = ~0; int rv = -7; /* VNET_API_ERROR_INVALID_VALUE */ @@ -84,10 +85,11 @@ vl_api_get_first_msg_id_t_handler (vl_api_get_first_msg_id_t * mp) if (!regp) return; + u8 *name = vl_api_from_api_to_vec (&mp->name); + if (am->msg_range_by_name == 0) goto out; - strncpy ((char *) name, (char *) mp->name, ARRAY_LEN (name)); - name[ARRAY_LEN (name) - 1] = '\0'; + p = hash_get_mem (am->msg_range_by_name, name); if (p == 0) goto out; @@ -97,6 +99,7 @@ vl_api_get_first_msg_id_t_handler (vl_api_get_first_msg_id_t * mp) rv = 0; out: + vec_free (name); rmp = vl_msg_api_alloc (sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_GET_FIRST_MSG_ID_REPLY); rmp->context = mp->context; @@ -133,10 +136,8 @@ vl_api_api_versions_t_handler (vl_api_api_versions_t * mp) rmp->api_versions[i].major = htonl (vl->major); rmp->api_versions[i].minor = htonl (vl->minor); rmp->api_versions[i].patch = htonl (vl->patch); - strncpy ((char *) rmp->api_versions[i].name, vl->name, - ARRAY_LEN (rmp->api_versions[i].name)); - rmp->api_versions[i].name[ARRAY_LEN (rmp->api_versions[i].name) - 1] = - '\0'; + vl_api_to_api_string (strnlen (vl->name, 64), vl->name, + &rmp->api_versions[i].name); } vl_api_send_msg (reg, (u8 *) rmp); @@ -193,8 +194,8 @@ send_one_plugin_msg_ids_msg (u8 * name, u16 first_msg_id, u16 last_msg_id) clib_memset (mp, 0, sizeof (*mp)); mp->_vl_msg_id = clib_host_to_net_u16 (VL_API_TRACE_PLUGIN_MSG_IDS); - strncpy ((char *) mp->plugin_name, (char *) name, - sizeof (mp->plugin_name) - 1); + vl_api_to_api_string (strnlen_s ((char *) name, 64), (char *) name, + &mp->plugin_name); mp->first_msg_id = clib_host_to_net_u16 (first_msg_id); mp->last_msg_id = clib_host_to_net_u16 (last_msg_id); @@ -625,11 +626,14 @@ vl_api_trace_plugin_msg_ids_t_handler (vl_api_trace_plugin_msg_ids_t * mp) if (am->replay_in_progress == 0) return; - p = hash_get_mem (am->msg_range_by_name, mp->plugin_name); + u8 *plugin_name = vl_api_from_api_to_vec (&mp->plugin_name); + vec_add1 (plugin_name, 0); + + p = hash_get_mem (am->msg_range_by_name, plugin_name); if (p == 0) { clib_warning ("WARNING: traced plugin '%s' not in current image", - mp->plugin_name); + plugin_name); return; } @@ -637,16 +641,17 @@ vl_api_trace_plugin_msg_ids_t_handler (vl_api_trace_plugin_msg_ids_t * mp) if (rp->first_msg_id != clib_net_to_host_u16 (mp->first_msg_id)) { clib_warning ("WARNING: traced plugin '%s' first message id %d not %d", - mp->plugin_name, clib_net_to_host_u16 (mp->first_msg_id), + plugin_name, clib_net_to_host_u16 (mp->first_msg_id), rp->first_msg_id); } if (rp->last_msg_id != clib_net_to_host_u16 (mp->last_msg_id)) { clib_warning ("WARNING: traced plugin '%s' last message id %d not %d", - mp->plugin_name, clib_net_to_host_u16 (mp->last_msg_id), + plugin_name, clib_net_to_host_u16 (mp->last_msg_id), rp->last_msg_id); } + vec_free (plugin_name); } #define foreach_rpc_api_msg \ diff --git a/src/vpp-api/vapi/vapi_c_gen.py b/src/vpp-api/vapi/vapi_c_gen.py index 381dcba7f42..047bb2c680a 100755 --- a/src/vpp-api/vapi/vapi_c_gen.py +++ b/src/vpp-api/vapi/vapi_c_gen.py @@ -14,7 +14,10 @@ class CField(Field): def get_c_def(self): if self.len is not None: - return "%s %s[%d];" % (self.type.get_c_name(), self.name, self.len) + try: + return "%s %s[%d];" % (self.type.get_c_name(), self.name, self.len) + except: + raise Exception("%s %s[%s];" % (self.type.get_c_name(), self.name, self.len)) else: return "%s %s;" % (self.type.get_c_name(), self.name) diff --git a/src/vpp-api/vapi/vapi_json_parser.py b/src/vpp-api/vapi/vapi_json_parser.py index a9d2c8186bc..d0f8de03617 100644 --- a/src/vpp-api/vapi/vapi_json_parser.py +++ b/src/vpp-api/vapi/vapi_json_parser.py @@ -167,48 +167,9 @@ class Message(object): else: field_type = json_parser.lookup_type_like_id(field[0]) logger.debug("Parsing message field `%s'" % field) - l = len(field) - if any(type(n) is dict for n in field): - l -= 1 - if l == 2: - if self.header is not None and\ - self.header.has_field(field[1]): - continue - p = field_class(field_name=field[1], - field_type=field_type) - elif l == 3: - if field[2] == 0: - raise ParseError( - "While parsing message `%s': variable length " - "array `%s' doesn't have reference to member " - "containing the actual length" % ( - name, field[1])) - p = field_class( - field_name=field[1], - field_type=field_type, - array_len=field[2]) - elif l == 4: - nelem_field = None - for f in fields: - if f.name == field[3]: - nelem_field = f - if nelem_field is None: - raise ParseError( - "While parsing message `%s': couldn't find " - "variable length array `%s' member containing " - "the actual length `%s'" % ( - name, field[1], field[3])) - p = field_class( - field_name=field[1], - field_type=field_type, - array_len=field[2], - nelem_field=nelem_field) - else: - raise Exception("Don't know how to parse message " - "definition for message `%s': `%s'" % - (m, m[1:])) - logger.debug("Parsed field `%s'" % p) - fields.append(p) + f = parse_field(field_class, fields, field, field_type) + logger.debug("Parsed field `%s'" % f) + fields.append(f) self.fields = fields self.depends = [f.type for f in self.fields] logger.debug("Parsed message: %s" % self) @@ -220,6 +181,48 @@ class Message(object): self.crc) +def parse_field(field_class, fields, field, field_type): + l = len(field) + if l > 2: + if type(field[2]) is dict: + if "limit" in field[2]: + array_len = field[2]["limit"] + else: + l -= 1 + else: + array_len = field[2] + + if l == 2: + return field_class(field_name=field[1], + field_type=field_type) + elif l == 3: + if field[2] == 0: + raise ParseError("While parsing type `%s': array `%s' has " + "variable length" % (name, field[1])) + return field_class(field_name=field[1], + field_type=field_type, + array_len=array_len) + elif l == 4: + nelem_field = None + for f in fields: + if f.name == field[3]: + nelem_field = f + if nelem_field is None: + raise ParseError( + "While parsing message `%s': couldn't find " + "variable length array `%s' member containing " + "the actual length `%s'" % ( + name, field[1], field[3])) + return field_class(field_name=field[1], + field_type=field_type, + array_len=array_len, + nelem_field=nelem_field) + else: + raise ParseError( + "Don't know how to parse field `%s' of type definition " + "for type `%s'" % (field, t)) + + class StructType (Type, Struct): def __init__(self, definition, json_parser, field_class, logger): @@ -233,36 +236,8 @@ class StructType (Type, Struct): continue field_type = json_parser.lookup_type_like_id(field[0]) logger.debug("Parsing type field `%s'" % field) - if len(field) == 2: - p = field_class(field_name=field[1], - field_type=field_type) - elif len(field) == 3: - if field[2] == 0: - raise ParseError("While parsing type `%s': array `%s' has " - "variable length" % (name, field[1])) - p = field_class(field_name=field[1], - field_type=field_type, - array_len=field[2]) - elif len(field) == 4: - nelem_field = None - for f in fields: - if f.name == field[3]: - nelem_field = f - if nelem_field is None: - raise ParseError( - "While parsing message `%s': couldn't find " - "variable length array `%s' member containing " - "the actual length `%s'" % ( - name, field[1], field[3])) - p = field_class(field_name=field[1], - field_type=field_type, - array_len=field[2], - nelem_field=nelem_field) - else: - raise ParseError( - "Don't know how to parse field `%s' of type definition " - "for type `%s'" % (field, t)) - fields.append(p) + f = parse_field(field_class, fields, field, field_type) + fields.append(f) Type.__init__(self, name) Struct.__init__(self, name, fields) diff --git a/src/vpp/api/custom_dump.c b/src/vpp/api/custom_dump.c index 24c37a6872b..403d434050c 100644 --- a/src/vpp/api/custom_dump.c +++ b/src/vpp/api/custom_dump.c @@ -2045,8 +2045,10 @@ static void *vl_api_memclnt_create_t_print (vl_api_memclnt_create_t * mp, void *handle) { u8 *s; + u8 *name = vl_api_from_api_to_vec (&mp->name); - s = format (0, "SCRIPT: memclnt_create name %s ", mp->name); + s = format (0, "SCRIPT: memclnt_create name %v ", name); + vec_free (name); FINISH; } @@ -2055,9 +2057,11 @@ static void *vl_api_sockclnt_create_t_print (vl_api_sockclnt_create_t * mp, void *handle) { u8 *s; + u8 *name = vl_api_from_api_to_vec (&mp->name); - s = format (0, "SCRIPT: sockclnt_create name %s ", mp->name); + s = format (0, "SCRIPT: sockclnt_create name %s ", name); + vec_free (name); FINISH; } diff --git a/test/ext/vapi_c_test.c b/test/ext/vapi_c_test.c index 725f5c30407..6e97b753b23 100644 --- a/test/ext/vapi_c_test.c +++ b/test/ext/vapi_c_test.c @@ -359,14 +359,14 @@ show_version_cb (vapi_ctx_t ctx, void *caller_ctx, { ck_assert_int_eq (VAPI_OK, rv); ck_assert_int_eq (true, is_last); - ck_assert_str_eq ("vpe", (char *) vl_api_from_api_string (&p->program)); + ck_assert_str_eq ("vpe", (char *) vl_api_from_api_string (p->program)); printf ("show_version_reply: program: `%s', version: `%s', build directory: " "`%s', build date: `%s'\n", - vl_api_from_api_string (&p->program), - vl_api_from_api_string (&p->version), - vl_api_from_api_string (&p->build_directory), - vl_api_from_api_string (&p->build_date)); + vl_api_from_api_string (p->program), + vl_api_from_api_string (p->version), + vl_api_from_api_string (p->build_directory), + vl_api_from_api_string (p->build_date)); ++*(int *) caller_ctx; return VAPI_OK; } @@ -803,8 +803,7 @@ generic_cb (vapi_ctx_t ctx, void *callback_ctx, vapi_msg_id_t id, void *msg) ck_assert_ptr_ne (NULL, msg); vapi_msg_show_version_reply *reply = msg; ck_assert_str_eq ("vpe", - (char *) vl_api_from_api_string (&reply-> - payload.program)); + (char *) vl_api_from_api_string (reply->payload.program)); return VAPI_OK; } diff --git a/test/ext/vapi_cpp_test.cpp b/test/ext/vapi_cpp_test.cpp index 46a2c0ec2f1..589eb94a5ed 100644 --- a/test/ext/vapi_cpp_test.cpp +++ b/test/ext/vapi_cpp_test.cpp @@ -49,11 +49,11 @@ void verify_show_version_reply (const Show_version_reply &r) auto &p = r.get_payload (); printf ("show_version_reply: program: `%s', version: `%s', build directory: " "`%s', build date: `%s'\n", - vl_api_from_api_string (&p.program), - vl_api_from_api_string (&p.version), - vl_api_from_api_string (&p.build_directory), - vl_api_from_api_string (&p.build_date)); - ck_assert_str_eq ("vpe", (char *)vl_api_from_api_string (&p.program)); + vl_api_from_api_string (p.program), + vl_api_from_api_string (p.version), + vl_api_from_api_string (p.build_directory), + vl_api_from_api_string (p.build_date)); + ck_assert_str_eq ("vpe", (char *)vl_api_from_api_string (p.program)); } Connection con; -- 2.16.6