From 96bf7aead524d2fac6ca8befce696efc4b4f0484 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Fri, 19 Jul 2019 11:54:39 -0500 Subject: [PATCH] bolt-gen: handle variable-sized and optionals actually do the right thing for variable-sized and optional field types --- tools/gen/header_template | 13 +++- tools/gen/impl_template | 112 ++++++++++++++++++++-------------- tools/gen/print_impl_template | 2 +- tools/generate-bolts.py | 47 ++++++++++---- 4 files changed, 112 insertions(+), 62 deletions(-) diff --git a/tools/gen/header_template b/tools/gen/header_template index 228d758c7..962684c3d 100644 --- a/tools/gen/header_template +++ b/tools/gen/header_template @@ -37,7 +37,9 @@ struct ${struct.struct_name()} { % if bool(f.len_field_of): <% continue %> % endif - % if f.has_len_field(): + % if f.is_varlen() and f.type_obj.is_varsize(): + ${f.type_obj.type_name()} **${f.name}; + % elif f.is_varlen(): ${f.type_obj.type_name()} *${f.name}; % elif f.is_array(): ${f.type_obj.type_name()} ${f.name}[${f.count}]; @@ -59,11 +61,16 @@ struct ${tlv.name} { % if options.expose_subtypes and bool(subtypes): % for subtype in subtypes: /* SUBTYPE: ${subtype.name.upper()} */ -void towire_${subtype.name}(u8 **p, const struct ${subtype.name} *${subtype.name}); -bool fromwire_${subtype.name}(${'const tal_t *ctx, ' if subtype.has_len_fields() else '' }const u8 **cursor, size_t *plen, struct ${subtype.name} *${subtype.name}); % for c in subtype.type_comments: /* ${c} */ % endfor +void towire_${subtype.name}(u8 **p, const ${subtype.type_name()} *${subtype.name}); +% if subtype.is_varsize(): +${static}${subtype.type_name()} * +fromwire_${subtype.name}(const tal_t *ctx, const u8 **cursor, size_t *plen); +% else: +void fromwire_${subtype.name}(const u8 **cursor, size_t *plen, ${subtype.type_name()} *${subtype.name}); +% endif % endfor % endif diff --git a/tools/gen/impl_template b/tools/gen/impl_template index 87c7eee47..d0d8a5ced 100644 --- a/tools/gen/impl_template +++ b/tools/gen/impl_template @@ -47,10 +47,10 @@ ${static}void towire_${subtype.name}(u8 **p, const struct ${subtype.name} *${sub % endfor <% fieldname = '{}->{}'.format(subtype.name,f.name) -%> \ - % if f.is_array() or f.has_len_field(): ## multiples? +%>\ + % if f.is_array() or f.is_varlen(): % if f.type_obj.has_array_helper(): - towire_${f.type_obj.name}_array(&p, ${fieldname}, ${f.size()}); + towire_${f.type_obj.name}_array(p, ${fieldname}, ${f.size()}); % else: for (size_t i = 0; i < ${f.size()}; i++) % if f.type_obj.is_assignable() or f.type_obj.has_len_fields(): @@ -66,8 +66,16 @@ ${static}void towire_${subtype.name}(u8 **p, const struct ${subtype.name} *${sub % endif % endfor } -${static}bool fromwire_${subtype.name}(${'const tal_t *ctx, ' if subtype.has_len_fields() else '' }const u8 **cursor, size_t *plen, struct ${subtype.name} *${subtype.name}) +% if subtype.is_varsize(): +${static}${subtype.type_name()} * +fromwire_${subtype.name}(const tal_t *ctx, const u8 **cursor, size_t *plen) +% else: +${static}void fromwire_${subtype.name}(${'const tal_t *ctx, ' if subtype.needs_context() else ''}const u8 **cursor, size_t *plen, ${subtype.type_name()} *${subtype.name}) +% endif { +% if subtype.is_varsize(): + ${subtype.type_name()} *${subtype.name} = tal(ctx, ${subtype.type_name()}); +% endif ## Length field declarations % for f in subtype.get_len_fields(): ${f.type_obj.type_name()} ${f.name}; @@ -79,28 +87,28 @@ ${static}bool fromwire_${subtype.name}(${'const tal_t *ctx, ' if subtype.has_len % endfor <% fieldname = '{}->{}'.format(subtype.name,f.name) - typename = f.type_obj.type_name() - type_ = f.type_obj.name -%> \ - % if f.has_len_field(): - ${fieldname} = ${f.len_field} ? tal_arr(ctx, ${typename}${' *' if f.type_obj.has_len_fields() else ''}, ${f.len_field}) : NULL; - % endif -<% + ctx = fieldname if f.is_array(): fieldname = '*' + fieldname - ctx = 'ctx' - else: - ctx = fieldname + if f.type_obj.is_varsize(): + typename += ' *' + type_ = f.type_obj.name + typename = f.type_obj.type_name() %> \ - % if f.is_array() or f.has_len_field(): + % if f.is_varlen(): + ${'*' if f.type_obj.is_varsize() else ''}${fieldname} = ${f.len_field} ? tal_arr(${subtype.name}, ${typename}, ${f.len_field}) : NULL; + % endif + % if f.is_array() or f.is_varlen(): % if f.type_obj.has_array_helper(): fromwire_${type_}_array(cursor, plen, ${fieldname}, ${f.size()}); % else: for (size_t i = 0; i < ${f.size()}; i++) % if f.type_obj.is_assignable(): (${fieldname})[i] = fromwire_${type_}(cursor, plen); - % elif f.has_len_field(): + % elif f.is_varlen() and f.type_obj.is_varsize(): (${fieldname})[i] = fromwire_${type_}(${ctx}, cursor, plen); + % elif f.is_varlen(): + fromwire_${type_}(cursor, plen, ${fieldname} + i); % else: fromwire_${type_}(${ctx}, cursor, plen, ${fieldname} + i); % endif @@ -108,17 +116,30 @@ ${static}bool fromwire_${subtype.name}(${'const tal_t *ctx, ' if subtype.has_len % else: % if f.type_obj.is_assignable(): ${ f.name if f.len_field_of else fieldname} = fromwire_${type_}(cursor, plen); + % elif f.type_obj.is_varsize(): + ${fieldname} = *fromwire_${type_}(ctx, cursor, plen); % else: fromwire_${type_}(cursor, plen, &${fieldname}); % endif %endif % endfor +% if subtype.is_varsize(): return ${subtype.name}; +% endif } - -% endfor +% endfor ## END Subtypes +<%def name="fromwire_phrase(f, type_, varsized)" >\ +%if f.type_obj.is_assignable(): +*${f.name} = fromwire_${type_}(&cursor, &plen); +% elif varsized: +*${f.name} = fromwire_${type_}(ctx, &cursor, &plen); +% else: +fromwire_${type_}(${'ctx, ' if f.needs_context() else ''}&cursor, &plen, ${'*' if f.is_optional else ''}${f.name}); +% endif + % for msg in messages: + /* WIRE: ${msg.name.upper()} */ % for c in msg.msg_comments: /* ${c} */ @@ -136,12 +157,12 @@ u8 *towire_${msg.name}(const tal_t *ctx${''.join([f.arg_desc_to() for f in msg.f % for c in f.field_comments: /* ${c} */ % endfor - % if f.is_array() or f.has_len_field(): ## multiples? + % if f.is_array() or f.is_varlen(): % if f.type_obj.has_array_helper(): towire_${f.type_obj.name}_array(&p, ${f.name}, ${f.size()}); % else: for (size_t i = 0; i < ${f.size()}; i++) - % if f.type_obj.is_assignable() or f.type_obj.has_len_fields(): + % if f.type_obj.is_assignable() or f.type_obj.is_varsize(): towire_${f.type_obj.name}(&p, ${f.name}[i]); % else: towire_${f.type_obj.name}(&p, ${f.name} + i); @@ -161,7 +182,7 @@ u8 *towire_${msg.name}(const tal_t *ctx${''.join([f.arg_desc_to() for f in msg.f return memcheck(p, tal_count(p)); } -bool fromwire_${msg.name}(${'const tal_t *ctx, ' if msg.has_len_fields() else ''}const void *p${''.join([f.arg_desc_from() for f in msg.fields.values() if not f.is_optional])}) +bool fromwire_${msg.name}(${'const tal_t *ctx, ' if msg.needs_context() else ''}const void *p${''.join([f.arg_desc_from() for f in msg.fields.values()])}) { % if msg.get_len_fields(): % for f in msg.get_len_fields(): @@ -178,51 +199,50 @@ bool fromwire_${msg.name}(${'const tal_t *ctx, ' if msg.has_len_fields() else '' % for f in msg.fields.values(): <% typename = f.type_obj.type_name() - if f.type_obj.has_len_fields(): + if f.type_obj.is_varsize(): typename = typename + ' *' type_ = f.type_obj.name + varsized = f.type_obj.is_varsize() %> \ % for c in f.field_comments: /* ${c} */ % endfor - % if f.has_len_field(): + % if f.is_varlen(): // 2nd case ${f.name} *${f.name} = ${f.len_field} ? tal_arr(ctx, ${typename}, ${f.len_field}) : NULL; % endif % if f.len_field_of: ${f.name} = fromwire_${type_}(&cursor, &plen); - % elif f.is_array() or f.has_len_field(): -<% - if f.has_len_field(): - fieldname = '*' + f.name - ctx = fieldname - else: - fieldname = f.name - ctx = 'ctx' - %> \ + % elif f.is_array() or f.is_varlen(): % if f.type_obj.has_array_helper(): - fromwire_${type_}_array(&cursor, &plen, ${fieldname}, ${f.size()}); + fromwire_${type_}_array(&cursor, &plen, ${'*' if f.is_varlen() else ''}${f.name}, ${f.size()}); % else: for (size_t i = 0; i < ${f.size()}; i++) - % if f.type_obj.is_assignable(): - (${fieldname})[i] = fromwire_${type_}(&cursor, &plen); - ## FIXME: case for 'varlen' structs - ## (${fieldname})[i] = fromwire_${type_}(${ctx}, &cursor, &plen); + % if not varsized and not f.type_obj.is_assignable(): + % if f.is_varlen(): + fromwire_${type_}(&cursor, &plen, *${f.name} + i); + % else: + fromwire_${type_}(&cursor, &plen, &(${f.name}[i])); + % endif % else: - fromwire_${type_}(${ctx + ', ' if f.type_obj.is_subtype() else ''}&cursor, &plen, ${fieldname} + i); + (${'' if f.type_obj.is_assignable() and f.is_array() else '*'}${f.name})[i] = fromwire_${type_}(${'*'+f.name+', ' if varsized else ''}&cursor, &plen); % endif % endif % else: -## FIXME: leaves out optional fields + 'varlen' structs - %if f.type_obj.is_assignable(): - *${f.name} = fromwire_${type_}(&cursor, &plen); - % else: - fromwire_${type_}(&cursor, &plen, ${f.name}); - ## assignment - % endif + % if not f.is_optional: + ${fromwire_phrase(f, type_, varsized)}\ + % else: ## Start optional + if (!fromwire_bool(&cursor, &plen)) + *${f.name} = NULL; + else { + % if not varsized: + *${f.name} = tal(ctx, ${typename}); + % endif + ${fromwire_phrase(f, type_, varsized)}\ + } + % endif ## End optional % endif % endfor return cursor != NULL; } - % endfor diff --git a/tools/gen/print_impl_template b/tools/gen/print_impl_template index 7d45b65c4..97fba0ebf 100644 --- a/tools/gen/print_impl_template +++ b/tools/gen/print_impl_template @@ -52,7 +52,7 @@ void print${options.enum_name}_tlv_message(const char *tlv_name, const u8 *msg) ${f.type_obj.type_name()} ${f.name} = fromwire_${f.type_obj.name}(${cursor}, ${plen});${truncate_check(nested)} <% continue %> \ % endif printf("${f.name}="); - % if f.is_array() or f.has_len_field(): + % if f.is_array() or f.is_varlen(): % if f.type_obj.has_array_helper(): printwire_${f.type_obj.name}_array(tal_fmt(NULL, "%s.${f.name}", fieldname), ${cursor}, ${plen}, ${f.size()}); % else: diff --git a/tools/generate-bolts.py b/tools/generate-bolts.py index 3b8632651..1661b62cc 100755 --- a/tools/generate-bolts.py +++ b/tools/generate-bolts.py @@ -63,7 +63,7 @@ class Field(object): def is_array(self): return self.count > 1 - def has_len_field(self): + def is_varlen(self): return not self.count def is_optional(self): @@ -77,17 +77,21 @@ class Field(object): return self.count return self.len_field + def needs_context(self): + """ A field needs a context if its type needs context + or if it's varsized """ + return self.is_varlen() or self.type_obj.needs_context() + def arg_desc_to(self): if self.len_field_of: return '' type_name = self.type_obj.type_name() if self.is_array(): return ', const {} {}[{}]'.format(type_name, self.name, self.count) - if self.type_obj.is_assignable() and not self.has_len_field(): + if self.type_obj.is_assignable() and not self.is_varlen(): return ', {} {}'.format(type_name, self.name) - # Are we a variable number of objects with a variable number of things? - if self.has_len_field() and self.type_obj.has_len_fields(): - return ', {} **{}'.format(type_name, self.name) + if self.is_varlen() and self.type_obj.is_varsize(): + return ', const {} **{}'.format(type_name, self.name) return ', const {} *{}'.format(type_name, self.name) def arg_desc_from(self): @@ -97,9 +101,9 @@ class Field(object): if self.is_array(): return ', {} {}[{}]'.format(type_name, self.name, self.count) ptrs = '*' - if self.has_len_field(): + if self.is_varlen() or self.is_optional or self.type_obj.is_varsize(): ptrs += '*' - if self.is_optional or self.type_obj.has_len_fields(): + if self.is_varlen() and self.type_obj.is_varsize(): ptrs += '*' return ', {} {}{}'.format(type_name, ptrs, self.name) @@ -139,6 +143,9 @@ class FieldSet(object): def has_len_fields(self): return bool(self.len_fields) + def needs_context(self): + return any([field.needs_context() for field in self.fields.values()]) + class Type(FieldSet): assignables = [ @@ -161,6 +168,18 @@ class Type(FieldSet): 'secp256k1_ecdsa_signature', ] + # Externally defined variable size types (require a context) + varsize_types = [ + 'peer_features', + 'gossip_getnodes_entry', + 'gossip_getchannels_entry', + 'failed_htlc', + 'utxo', + 'bitcoin_tx', + 'wirestring', + 'per_peer_state', + ] + # Some BOLT types are re-typed based on their field name # ('fieldname partial', 'original type'): ('true type', 'collapse array?') name_field_map = { @@ -229,19 +248,23 @@ class Type(FieldSet): return self.name def subtype_deps(self): - return [dep for dep in self.depends_on.values() if dep.is_gen_subtype()] + return [dep for dep in self.depends_on.values() if dep.is_subtype()] - def is_gen_subtype(self): - """ is this a 'genuine' subtype; i.e. will be generated """ + def is_subtype(self): return bool(self.fields) - def is_subtype(self): - return self.is_gen_subtype() + def needs_context(self): + return self.is_varsize() or any([field.needs_context() for field in self.fields.values()]) def is_assignable(self): """ Generally typedef's and enums """ return self.name in self.assignables or self.is_enum + def is_varsize(self): + """ A type is variably sized if it's marked as such (in varsize_types) + or it contains a field of variable length """ + return self.name in self.varsize_types or self.has_len_fields() + def add_comments(self, comments): self.type_comments = comments