From 036c1eacb540b331e2ac615250ba8ea392791784 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Tue, 2 Apr 2019 19:28:44 -0700 Subject: [PATCH 01/21] tlv: add tlv field's type enums to header file Version 1.1 of the lightning-rfc spec introduces TLVs for optional data fields. This starts the process of updating our auto-gen'd wireformat parsers to be able to understand TLV fields. The general way to declare a new TLV field is to add a '+' to the end of the fieldname. All field type declarations for that TLV set should be added to a file in the same directory by the name `gen__csv`. Note that the FIXME included in this commit is difficult to fix, as we currently pass in the csv files via stdin (so there's no easy way to ascertain the originating directory of file) --- tools/generate-wire.py | 93 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 394935293dd7..ee0a5ce9507e 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -126,6 +126,16 @@ def __init__(self, message, name, size, comments, prevname): self.lenvar = None self.num_elems = 1 self.optional = False + self.is_tlv = False + + # field name appended with '+' means this field contains a tlv + if name.endswith('+'): + self.is_tlv = True + self.name = name[:-1] + if self.name not in tlv_fields: + # FIXME: use the rest of this + tlv_includes, tlv_messages, tlv_comments = parse_tlv_file(self.name) + tlv_fields[self.name] = tlv_messages # ? means optional field (not supported for arrays) if size.startswith('?'): @@ -630,6 +640,43 @@ def find_message_with_option(messages, optional_messages, name, option): return m +def get_directory_prefix(): + # FIXME: use prefix of filename + return "wire/" + + +def get_tlv_filename(field_name): + return 'gen_{}_csv'.format(field_name) + + +def parse_tlv_file(tlv_field_name): + tlv_includes = [] + tlv_messages = [] + tlv_comments = [] + with open(get_directory_prefix() + get_tlv_filename(tlv_field_name)) as f: + for line in f: + # #include gets inserted into header + if line.startswith('#include '): + tlv_includes.append(line) + continue + + by_comments = line.rstrip().split('#') + + # Emit a comment if they included one + if by_comments[1:]: + tlv_comments.append(' '.join(by_comments[1:])) + + parts = by_comments[0].split(',') + if parts == ['']: + continue + + if len(parts) == 2: + # eg commit_sig,132 + tlv_messages.append(Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments)) + tlv_comments = [] + return tlv_includes, tlv_messages, tlv_comments + + parser = argparse.ArgumentParser(description='Generate C from CSV') parser.add_argument('--header', action='store_true', help="Create wire header") parser.add_argument('--bolt', action='store_true', help="Generate wire-format for BOLT") @@ -644,6 +691,7 @@ def find_message_with_option(messages, optional_messages, name, option): messages_with_option = [] comments = [] includes = [] +tlv_fields = {} prevfield = None # Read csv lines. Single comma is the message values, more is offset/len. @@ -690,6 +738,38 @@ def find_message_with_option(messages, optional_messages, name, option): prevfield = parts[2] comments = [] + +def construct_enums(msgs): + enums = "" + for m in msgs: + for c in m.comments: + enums += '\t/*{} */\n'.format(c) + enums += '\t{} = {},\n'.format(m.enum.name, m.enum.value) + return enums + + +def enum_header(enums, enumname): + return enum_header_template.format( + enums=enums, + enumname=enumname) + + +def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): + enum_set = "" + enum_set += enum_header(toplevel_enums, toplevel_enumname) + for field_name, messages in tlv_fields.items(): + enum_set += "\n" + enums = construct_enums(messages) + enum_set += enum_header(enums, field_name + '_type') + return enum_set + + +enum_header_template = """enum {enumname} {{ +{enums} +}}; +const char *{enumname}_name(int e); +""" + header_template = """/* This file was generated by generate-wire.py */ /* Do not modify this file! Modify the _csv file it was generated from. */ #ifndef LIGHTNING_{idem} @@ -697,10 +777,7 @@ def find_message_with_option(messages, optional_messages, name, option): #include #include {includes} -enum {enumname} {{ -{enums}}}; -const char *{enumname}_name(int e); - +{formatted_enums} {func_decls} #endif /* LIGHTNING_{idem} */ """ @@ -773,11 +850,8 @@ def find_message_with_option(messages, optional_messages, name, option): template = impl_template # Dump out enum, sorted by value order. -enums = "" -for m in messages: - for c in m.comments: - enums += '\t/*{} */\n'.format(c) - enums += '\t{} = {},\n'.format(m.enum.name, m.enum.value) +enums = construct_enums(messages) +built_enums = build_enums(options.enumname, enums, tlv_fields) includes = '\n'.join(includes) cases = ['case {enum.name}: return "{enum.name}";'.format(enum=m.enum) for m in messages] printcases = ['case {enum.name}: printf("{enum.name}:\\n"); printwire_{name}("{name}", msg); return;'.format(enum=m.enum, name=m.name) for m in messages] @@ -797,4 +871,5 @@ def find_message_with_option(messages, optional_messages, name, option): includes=includes, enumname=options.enumname, enums=enums, + formatted_enums=built_enums, func_decls='\n'.join(decls))) From e3db317e9c92f2d7afeb4e375f23eb89aca60aa8 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:17:07 -0700 Subject: [PATCH 02/21] tlv: add tlv fields to enum declarations in implementation file '.c' wire format files include case statements to print the names of enums. Include such methods for the enums pertaining to tlv's as well. --- tools/generate-wire.py | 97 +++++++++++++++++++++++++++++++----------- 1 file changed, 71 insertions(+), 26 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index ee0a5ce9507e..e5489ea7f101 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -248,6 +248,31 @@ def _guess_type(message, fieldname, base_size): }} """ +towire_tlv_templ = """u8 *towire_{name}(const tal_t *ctx{args}) +{{ +{field_decls} +\tu8 *p = tal_arr(ctx, u8, 0); +\ttowire_u16(&p, {enumname}); +\ttowire_u16(&p, {len}); +{subcalls} + +\treturn memcheck(p, tal_count(p)); +}} +""" + +fromwire_tlv_templ = """bool frowire_{name}({ctx}const void *p{args}) +{{ +{fields} +\tconst u8 *cursor = p; +\tsize_t plen = tal_count(p); + +\tif (frmwire_u16(&cursor, &plen) != {enum.name}) +\t\treturn false; +{subcalls} +\treturn cursor != NULL; +}} +""" + printwire_header_templ = """void printwire_{name}(const char *fieldname, const u8 *cursor); """ printwire_impl_templ = """void printwire_{name}(const char *fieldname, const u8 *cursor) @@ -739,7 +764,7 @@ def parse_tlv_file(tlv_field_name): comments = [] -def construct_enums(msgs): +def construct_hdr_enums(msgs): enums = "" for m in msgs: for c in m.comments: @@ -748,19 +773,39 @@ def construct_enums(msgs): return enums +def construct_impl_enums(msgs): + return '\n\t'.join(['case {enum.name}: return "{enum.name}";'.format(enum=m.enum) for m in msgs]) + + def enum_header(enums, enumname): - return enum_header_template.format( + return format_enums(enum_header_template, enums, enumname) + + +def enum_impl(enums, enumname): + return format_enums(enum_impl_template, enums, enumname) + + +def format_enums(template, enums, enumname): + return template.format( enums=enums, enumname=enumname) -def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): +def build_hdr_enums(toplevel_enumname, messages, tlv_fields): enum_set = "" - enum_set += enum_header(toplevel_enums, toplevel_enumname) + enum_set += enum_header(construct_hdr_enums(messages), toplevel_enumname) for field_name, messages in tlv_fields.items(): enum_set += "\n" - enums = construct_enums(messages) - enum_set += enum_header(enums, field_name + '_type') + enum_set += enum_header(construct_hdr_enums(messages), field_name + '_type') + return enum_set + + +def build_impl_enums(toplevel_enumname, messages, tlv_fields): + enum_set = "" + enum_set += enum_impl(construct_impl_enums(messages), toplevel_enumname) + for field_name, messages in tlv_fields.items(): + enum_set += "\n" + enum_set += enum_impl(construct_impl_enums(messages), field_name + '_type') return enum_set @@ -770,6 +815,20 @@ def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): const char *{enumname}_name(int e); """ +enum_impl_template = """ +const char *{enumname}_name(int e) +{{ +\tstatic char invalidbuf[sizeof("INVALID ") + STR_MAX_CHARS(e)]; + +\tswitch ((enum {enumname})e) {{ +\t{enums} +\t}} + +\tsnprintf(invalidbuf, sizeof(invalidbuf), "INVALID %i", e); +\treturn invalidbuf; +}} +""" + header_template = """/* This file was generated by generate-wire.py */ /* Do not modify this file! Modify the _csv file it was generated from. */ #ifndef LIGHTNING_{idem} @@ -777,7 +836,7 @@ def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): #include #include {includes} -{formatted_enums} +{formatted_hdr_enums} {func_decls} #endif /* LIGHTNING_{idem} */ """ @@ -788,19 +847,7 @@ def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): #include #include #include - -const char *{enumname}_name(int e) -{{ -\tstatic char invalidbuf[sizeof("INVALID ") + STR_MAX_CHARS(e)]; - -\tswitch ((enum {enumname})e) {{ -\t{cases} -\t}} - -\tsnprintf(invalidbuf, sizeof(invalidbuf), "INVALID %i", e); -\treturn invalidbuf; -}} - +{formatted_impl_enums} {func_decls} """ @@ -850,10 +897,9 @@ def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): template = impl_template # Dump out enum, sorted by value order. -enums = construct_enums(messages) -built_enums = build_enums(options.enumname, enums, tlv_fields) +built_hdr_enums = build_hdr_enums(options.enumname, messages, tlv_fields) +built_impl_enums = build_impl_enums(options.enumname, messages, tlv_fields) includes = '\n'.join(includes) -cases = ['case {enum.name}: return "{enum.name}";'.format(enum=m.enum) for m in messages] printcases = ['case {enum.name}: printf("{enum.name}:\\n"); printwire_{name}("{name}", msg); return;'.format(enum=m.enum, name=m.name) for m in messages] if options.printwire: @@ -865,11 +911,10 @@ def build_enums(toplevel_enumname, toplevel_enums, tlv_fields): print(template.format( headerfilename=options.headerfilename, - cases='\n\t'.join(cases), printcases='\n\t'.join(printcases), idem=idem, includes=includes, enumname=options.enumname, - enums=enums, - formatted_enums=built_enums, + formatted_hdr_enums=built_hdr_enums, + formatted_impl_enums=built_impl_enums, func_decls='\n'.join(decls))) From 5b89270f71324cb2b5d529c618cda42bcbdcc79e Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Mon, 11 Mar 2019 18:18:52 -0700 Subject: [PATCH 03/21] tlv: parse fields for tlv messages --- tools/generate-wire.py | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index e5489ea7f101..6fffe73f0a77 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -678,6 +678,7 @@ def parse_tlv_file(tlv_field_name): tlv_includes = [] tlv_messages = [] tlv_comments = [] + tlv_prevfield = None with open(get_directory_prefix() + get_tlv_filename(tlv_field_name)) as f: for line in f: # #include gets inserted into header @@ -697,7 +698,31 @@ def parse_tlv_file(tlv_field_name): if len(parts) == 2: # eg commit_sig,132 - tlv_messages.append(Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments)) + tlv_msg = Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments) + tlv_messages.append(tlv_msg) + + tlv_comments = [] + tlv_prevfield = None + else: + if len(parts) == 4: + # eg commit_sig,0,channel-id,8 OR + # commit_sig,0,channel-id,u64 + m = find_message(tlv_messages, parts[0]) + if m is None: + raise ValueError('Unknown message {}'.format(parts[0])) + elif len(parts) == 5: + # eg. + # channel_reestablish,48,your_last_per_commitment_secret,32,option209 + m = find_message_with_option(tlv_messages, messages_with_option, parts[0], parts[4]) + else: + raise ValueError('Line {} malformed'.format(line.rstrip())) + + f = Field(m.name, parts[2], parts[3], tlv_comments, tlv_prevfield) + m.addField(f) + # If it used prevfield as lenvar, keep that for next + # time (multiple fields can use the same lenvar). + if not f.lenvar: + tlv_prevfield = parts[2] tlv_comments = [] return tlv_includes, tlv_messages, tlv_comments From ac951a6e16e36f1dfc621391e6fa2277ae0786dd Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:18:16 -0700 Subject: [PATCH 04/21] tlv: add tlv messages to general message set Add tlv-messages to the general messages set so that their parsing messages get printed out. FIXME: figure out how to account for partial message length processing? --- tools/generate-wire.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 6fffe73f0a77..b4d0a0feddbf 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -330,12 +330,13 @@ def __str__(self): class Message(object): - def __init__(self, name, enum, comments): + def __init__(self, name, enum, comments, is_tlv): self.name = name self.enum = enum self.comments = comments self.fields = [] self.has_variable_fields = False + self.is_tlv = is_tlv def checkLenField(self, field): # Optional fields don't have a len. @@ -698,8 +699,9 @@ def parse_tlv_file(tlv_field_name): if len(parts) == 2: # eg commit_sig,132 - tlv_msg = Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments) + tlv_msg = Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments, True) tlv_messages.append(tlv_msg) + messages.append(tlv_msg) tlv_comments = [] tlv_prevfield = None @@ -763,7 +765,7 @@ def parse_tlv_file(tlv_field_name): if len(parts) == 2: # eg commit_sig,132 - messages.append(Message(parts[0], Enumtype("WIRE_" + parts[0].upper(), parts[1]), comments)) + messages.append(Message(parts[0], Enumtype("WIRE_" + parts[0].upper(), parts[1]), comments, False)) comments = [] prevfield = None else: @@ -816,21 +818,21 @@ def format_enums(template, enums, enumname): enumname=enumname) -def build_hdr_enums(toplevel_enumname, messages, tlv_fields): +def build_hdr_enums(toplevel_enumname, toplevel_messages, tlv_fields): enum_set = "" - enum_set += enum_header(construct_hdr_enums(messages), toplevel_enumname) - for field_name, messages in tlv_fields.items(): + enum_set += enum_header(construct_hdr_enums(toplevel_messages), toplevel_enumname) + for field_name, tlv_messages in tlv_fields.items(): enum_set += "\n" - enum_set += enum_header(construct_hdr_enums(messages), field_name + '_type') + enum_set += enum_header(construct_hdr_enums(tlv_messages), field_name + '_type') return enum_set -def build_impl_enums(toplevel_enumname, messages, tlv_fields): +def build_impl_enums(toplevel_enumname, toplevel_messages, tlv_fields): enum_set = "" - enum_set += enum_impl(construct_impl_enums(messages), toplevel_enumname) - for field_name, messages in tlv_fields.items(): + enum_set += enum_impl(construct_impl_enums(toplevel_messages), toplevel_enumname) + for field_name, tlv_messages in tlv_fields.items(): enum_set += "\n" - enum_set += enum_impl(construct_impl_enums(messages), field_name + '_type') + enum_set += enum_impl(construct_impl_enums(tlv_messages), field_name + '_type') return enum_set @@ -921,11 +923,12 @@ def build_impl_enums(toplevel_enumname, messages, tlv_fields): else: template = impl_template -# Dump out enum, sorted by value order. -built_hdr_enums = build_hdr_enums(options.enumname, messages, tlv_fields) -built_impl_enums = build_impl_enums(options.enumname, messages, tlv_fields) +# Print out all the things +toplevel_messages = [m for m in messages if not m.is_tlv] +built_hdr_enums = build_hdr_enums(options.enumname, toplevel_messages, tlv_fields) +built_impl_enums = build_impl_enums(options.enumname, toplevel_messages, tlv_fields) includes = '\n'.join(includes) -printcases = ['case {enum.name}: printf("{enum.name}:\\n"); printwire_{name}("{name}", msg); return;'.format(enum=m.enum, name=m.name) for m in messages] +printcases = ['case {enum.name}: printf("{enum.name}:\\n"); printwire_{name}("{name}", msg); return;'.format(enum=m.enum, name=m.name) for m in toplevel_messages] if options.printwire: decls = [m.print_printwire(options.header) for m in messages + messages_with_option] From cdcb9ff9222e92eb038eccdcfbd42b5a6d971851 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:19:12 -0700 Subject: [PATCH 05/21] tlv: add structs for message types to wire format .h's (header files) Since messages in TLV's are optional, the ideal way to deal with them is to have a 'master struct' object for every defined tlv, where the presence or lack of a field can be determined via the presence (or lack thereof) of a struct for each of the optional message types. In order to do this, appropriately, we need a struct for every TLV message. The next commit will make use of these. Note that right now TLV message structs aren't namespaced to the TLV they belong to, so there's the potential for collision. This should be fixed when/where it occurs (should fail to compile). --- tools/generate-wire.py | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index b4d0a0feddbf..02e5bc4105f1 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -641,6 +641,35 @@ def print_printwire(self, is_header): subcalls=str(subcalls) ) + def print_struct(self): + """ returns a string representation of this message as + a struct""" + if not self.is_tlv: + raise TypeError('{} is not a TLV-message').format(self.name) + + fmt_fields = CCode() + for f in self.fields: + if f.is_len_var or f.is_padding(): + # there is no ethical padding under TLVs + continue + elif f.is_variable_size(): + fmt_fields.append('{} *{};'.format(f.fieldtype.name, f.name)) + elif f.is_array(): + fmt_fields.append('{} {}[{}];'.format(f.fieldtype.name, f.name, f.num_elems)) + else: + fmt_fields.append('{} {};'.format(f.fieldtype.name, f.name)) + + return tlv_struct_template.format( + tlv_name=self.name, + fields=str(fmt_fields)) + + +tlv_struct_template = """ +struct tlv_{tlv_name} {{ +{fields} +}}; +""" + def find_message(messages, name): for m in messages: @@ -836,6 +865,14 @@ def build_impl_enums(toplevel_enumname, toplevel_messages, tlv_fields): return enum_set +def build_tlv_structs(tlv_fields): + structs = "" + for field_name, tlv_messages in tlv_fields.items(): + for m in tlv_messages: + structs += m.print_struct() + return structs + + enum_header_template = """enum {enumname} {{ {enums} }}; @@ -863,7 +900,7 @@ def build_impl_enums(toplevel_enumname, toplevel_messages, tlv_fields): #include #include {includes} -{formatted_hdr_enums} +{formatted_hdr_enums}{tlv_structs} {func_decls} #endif /* LIGHTNING_{idem} */ """ @@ -927,6 +964,7 @@ def build_impl_enums(toplevel_enumname, toplevel_messages, tlv_fields): toplevel_messages = [m for m in messages if not m.is_tlv] built_hdr_enums = build_hdr_enums(options.enumname, toplevel_messages, tlv_fields) built_impl_enums = build_impl_enums(options.enumname, toplevel_messages, tlv_fields) +tlv_structs = build_tlv_structs(tlv_fields) includes = '\n'.join(includes) printcases = ['case {enum.name}: printf("{enum.name}:\\n"); printwire_{name}("{name}", msg); return;'.format(enum=m.enum, name=m.name) for m in toplevel_messages] @@ -945,4 +983,5 @@ def build_impl_enums(toplevel_enumname, toplevel_messages, tlv_fields): enumname=options.enumname, formatted_hdr_enums=built_hdr_enums, formatted_impl_enums=built_impl_enums, + tlv_structs=tlv_structs, func_decls='\n'.join(decls))) From ea5182bb13fc5f8700f610d893657e75e4f6984d Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:20:39 -0700 Subject: [PATCH 06/21] tlv: build tlv top-level structs construct structs for the TLV's. these will be the 'return type' for tlv fields in parent messages (so to speak) --- tools/generate-wire.py | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 02e5bc4105f1..d849886ada5b 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -659,17 +659,39 @@ def print_struct(self): else: fmt_fields.append('{} {};'.format(f.fieldtype.name, f.name)) - return tlv_struct_template.format( - tlv_name=self.name, + return tlv_msg_struct_template.format( + msg_name=self.name, fields=str(fmt_fields)) -tlv_struct_template = """ -struct tlv_{tlv_name} {{ +tlv_msg_struct_template = """ +struct _tlv_msg_{msg_name} {{ {fields} }}; """ +tlv_struct_template = """ +struct _{tlv_name} {{ +{msg_type_structs} +}}; +""" + + +def build_tlv_type_struct(name, messages): + inner_structs = CCode() + for m in messages: + inner_structs.append('struct _tlv_msg_{} *{};'.format(m.name, m.name)) + return tlv_struct_template.format( + tlv_name=name, + msg_type_structs=str(inner_structs)) + + +def build_tlv_type_structs(tlv_fields): + structs = '' + for name, messages in tlv_fields.items(): + structs += build_tlv_type_struct(name, messages) + return structs + def find_message(messages, name): for m in messages: @@ -965,6 +987,7 @@ def build_tlv_structs(tlv_fields): built_hdr_enums = build_hdr_enums(options.enumname, toplevel_messages, tlv_fields) built_impl_enums = build_impl_enums(options.enumname, toplevel_messages, tlv_fields) tlv_structs = build_tlv_structs(tlv_fields) +tlv_structs += build_tlv_type_structs(tlv_fields) includes = '\n'.join(includes) printcases = ['case {enum.name}: printf("{enum.name}:\\n"); printwire_{name}("{name}", msg); return;'.format(enum=m.enum, name=m.name) for m in toplevel_messages] From 63275ef1c1fecec1c7f3a29151225f84d119c41a Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:23:18 -0700 Subject: [PATCH 07/21] tlv: add fromwire_ methods for TLV structs --- tools/generate-wire.py | 277 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 262 insertions(+), 15 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index d849886ada5b..eee5d8ea4c34 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -232,6 +232,21 @@ def _guess_type(message, fieldname, base_size): }} """ +fromwire_tlv_impl_templ = """static bool _fromwire_{tlv_name}_{name}({ctx}{args}) +{{ + +\tsize_t start_len, plen; +{fields} +\tconst u8 *cursor = p; +\tplen = tal_count(p); +\tif (plen < len) +\t\treturn false; +\tstart_len = plen; +{subcalls} +\treturn cursor != NULL && (start_len - plen == len); +}} +""" + fromwire_header_templ = """bool fromwire_{name}({ctx}const void *p{args}); """ @@ -384,7 +399,72 @@ def print_fromwire_array(self, ctx, subcalls, basetype, f, name, num_elems): subcalls.append('fromwire_{}(&cursor, &plen, {} + i);' .format(basetype, name)) - def print_fromwire(self, is_header): + def print_tlv_fromwire(self, tlv_name): + """ prints fromwire function definition for a TLV message. + these are significantly different in that they take in a struct + to populate, instead of fields, as well as a length to read in + """ + ctx_arg = 'const tal_t *ctx, ' if self.has_variable_fields else '' + args = 'const void *p, const u16 len, struct _tlv_msg_{name} *{name}'.format(name=self.name) + fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] + subcalls = CCode() + for f in self.fields: + basetype = f.basetype() + if f.is_tlv: + raise TypeError('Nested TLVs arent allowed!!') + elif f.optional: + raise TypeError('Optional fields on TLV messages not currently supported') + + for c in f.comments: + subcalls.append('/*{} */'.format(c)) + + if f.is_padding(): + subcalls.append('fromwire_pad(&cursor, &plen, {});' + .format(f.num_elems)) + elif f.is_array(): + name = '*{}->{}'.format(self.name, f.name) + self.print_fromwire_array('ctx', subcalls, basetype, f, name, + f.num_elems) + elif f.is_variable_size(): + subcalls.append("// 2nd case {name}".format(name=f.name)) + typename = f.fieldtype.name + # If structs are varlen, need array of ptrs to them. + if basetype in varlen_structs: + typename += ' *' + subcalls.append('{}->{} = {} ? tal_arr(ctx, {}, {}) : NULL;' + .format(self.name, f.name, f.lenvar, typename, f.lenvar)) + + name = '{}->{}'.format(self.name, f.name) + # Allocate these off the array itself, if they need alloc. + self.print_fromwire_array('*' + f.name, subcalls, basetype, f, + name, f.lenvar) + else: + if f.is_assignable(): + if f.is_len_var: + s = '{} = fromwire_{}(&cursor, &plen);'.format(f.name, basetype) + else: + s = '{}->{} = fromwire_{}(&cursor, &plen);'.format( + self.name, f.name, basetype) + else: + s = 'fromwire_{}(&cursor, &plen, *{}->{});'.format( + basetype, self.name, f.name) + subcalls.append(s) + + return fromwire_tlv_impl_templ.format( + tlv_name=tlv_name, + name=self.name, + ctx=ctx_arg, + args=''.join(args), + fields=''.join(fields), + subcalls=str(subcalls) + ) + + def print_fromwire(self, is_header, tlv_name): + if self.is_tlv: + if is_header: + return '' + return self.print_tlv_fromwire(tlv_name) + ctx_arg = 'const tal_t *ctx, ' if self.has_variable_fields else '' args = [] @@ -394,6 +474,8 @@ def print_fromwire(self, is_header): continue elif f.is_array(): args.append(', {} {}[{}]'.format(f.fieldtype.name, f.name, f.num_elems)) + elif f.is_tlv: + args.append(', struct _{} *{}'.format(f.name, f.name)) else: ptrs = '*' # If we're handing a variable array, we need a ptr-to-ptr. @@ -421,6 +503,12 @@ def print_fromwire(self, is_header): elif f.is_array(): self.print_fromwire_array('ctx', subcalls, basetype, f, f.name, f.num_elems) + elif f.is_tlv: + if not f.is_variable_size(): + raise TypeError('TLV {} not variable size'.format(f.name)) + subcalls.append('if (!fromwire__{tlv_name}(ctx, &cursor, &{tlv_len}, {tlv_name}))' + .format(tlv_name=f.name, tlv_len=f.lenvar)) + subcalls.append('return false;') elif f.is_variable_size(): subcalls.append("//2nd case {name}".format(name=f.name)) typename = f.fieldtype.name @@ -472,21 +560,67 @@ def print_fromwire(self, is_header): subcalls=str(subcalls) ) - def print_towire_array(self, subcalls, basetype, f, num_elems): + def print_towire_array(self, subcalls, basetype, f, num_elems, is_tlv=False): + p_ref = '' if is_tlv else '&' + msg_name = self.name + '->' if is_tlv else '' if f.has_array_helper(): - subcalls.append('towire_{}_array(&p, {}, {});' - .format(basetype, f.name, num_elems)) + subcalls.append('towire_{}_array({}p, {}{}, {});' + .format(basetype, p_ref, msg_name, f.name, num_elems)) else: subcalls.append('for (size_t i = 0; i < {}; i++)' .format(num_elems)) if f.fieldtype.is_assignable() or basetype in varlen_structs: - subcalls.append('towire_{}(&p, {}[i]);' - .format(basetype, f.name)) + subcalls.append('towire_{}({}p, {}{}[i]);' + .format(basetype, p_ref, msg_name, f.name)) else: - subcalls.append('towire_{}(&p, {} + i);' - .format(basetype, f.name)) + subcalls.append('towire_{}({}p, {}{} + i);' + .format(basetype, p_ref, msg_name, f.name)) - def print_towire(self, is_header): + def print_tlv_towire(self, tlv_name): + """ prints towire function definition for a TLV message.""" + field_decls = [] + for f in self.fields: + if f.is_tlv: + raise TypeError("Nested TLVs aren't allowed!! {}->{}".format(tlv_name, f.name)) + elif f.optional: + raise TypeError("Optional fields on TLV messages not currently supported. {}->{}".format(tlv_name, f.name)) + if f.is_len_var: + field_decls.append('\t{0} {1} = tal_count(&{2}->{3});'.format( + f.fieldtype.name, f.name, self.name, f.lenvar_for.name + )) + + subcalls = CCode() + for f in self.fields: + basetype = f.fieldtype.name + if basetype.startswith('struct '): + basetype = basetype[7:] + elif basetype.startswith('enum '): + basetype = basetype[5:] + + for c in f.comments: + subcalls.append('/*{} */'.format(c)) + + if f.is_padding(): + subcalls.append('towire_pad(p, {});'.format(f.num_elems)) + elif f.is_array(): + self.print_towire_array(subcalls, basetype, f, f.num_elems, is_tlv=True) + elif f.is_variable_size(): + self.print_towire_array(subcalls, basetype, f, f.lenvar, is_tlv=True) + elif f.is_len_var: + subcalls.append('towire_{}(p, {});'.format(basetype, f.name)) + else: + subcalls.append('towire_{}(p, {}->{});'.format(basetype, self.name, f.name)) + return tlv_message_towire_stub.format( + tlv_name=tlv_name, + name=self.name, + field_decls='\n'.join(field_decls), + subcalls=str(subcalls)) + + def print_towire(self, is_header, tlv_name): + if self.is_tlv: + if is_header: + return '' + return self.print_tlv_towire(tlv_name) template = towire_header_templ if is_header else towire_impl_templ args = [] for f in self.fields: @@ -494,6 +628,8 @@ def print_towire(self, is_header): continue if f.is_array(): args.append(', const {} {}[{}]'.format(f.fieldtype.name, f.name, f.num_elems)) + elif f.is_tlv: + args.append(', const struct _{} *{}'.format(f.name, f.name)) elif f.is_assignable(): args.append(', {} {}'.format(f.fieldtype.name, f.name)) elif f.is_variable_size() and f.basetype() in varlen_structs: @@ -504,9 +640,14 @@ def print_towire(self, is_header): field_decls = [] for f in self.fields: if f.is_len_var: - field_decls.append('\t{0} {1} = tal_count({2});'.format( - f.fieldtype.name, f.name, f.lenvar_for.name - )) + if f.lenvar_for.is_tlv: + field_decls.append('\t{0} {1} = sizeof({2});'.format( + f.fieldtype.name, f.name, f.lenvar_for.name + )) + else: + field_decls.append('\t{0} {1} = tal_count({2});'.format( + f.fieldtype.name, f.name, f.lenvar_for.name + )) subcalls = CCode() for f in self.fields: @@ -524,6 +665,11 @@ def print_towire(self, is_header): .format(f.num_elems)) elif f.is_array(): self.print_towire_array(subcalls, basetype, f, f.num_elems) + elif f.is_tlv: + if not f.is_variable_size(): + raise TypeError('TLV {} not variable size'.format(f.name)) + subcalls.append('towire__{tlv_name}(&p, {tlv_name});'.format( + tlv_name=f.name)) elif f.is_variable_size(): self.print_towire_array(subcalls, basetype, f, f.lenvar) else: @@ -664,6 +810,13 @@ def print_struct(self): fields=str(fmt_fields)) +tlv_message_towire_stub = """static void _towire_{tlv_name}_{name}(u8 **p, struct _tlv_msg_{name} *{name}) {{ +{field_decls} +{subcalls} +}} +""" + + tlv_msg_struct_template = """ struct _tlv_msg_{msg_name} {{ {fields} @@ -676,6 +829,87 @@ def print_struct(self): }}; """ +tlv__type_impl_towire_fields = """\tif ({tlv_name}->{name}) {{ +\t\ttowire_u16(p, {enum}); +\t\ttowire_u16(p, sizeof(*{tlv_name}->{name})); +\t\t_towire_{tlv_name}_{name}(p, {tlv_name}->{name}); +\t}} +""" + +tlv__type_impl_towire_template = """static void towire__{tlv_name}(u8 **p, const struct _{tlv_name} *{tlv_name}) {{ +{fields}}} +""" + +tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, const u16 *len, struct _{tlv_name} *{tlv_name}) {{ +\tu16 msg_type, msg_len; +\tconst u8 *cursor = *p; +\tsize_t plen = tal_count(p); +\tif (plen != *len) +\t\treturn false; + +\twhile (cursor && plen) {{ +\t\tmsg_type = fromwire_u16(&cursor, &plen); +\t\tmsg_len = fromwire_u16(&cursor, &plen); +\t\tif (plen < msg_len) {{ +\t\t\tfromwire_fail(&cursor, &plen); +\t\t\tbreak; +\t\t}} +\t\tswitch((enum {tlv_name}_type)msg_type) {{ +{cases}\t\tdefault: +\t\t\t// FIXME: print a warning / message? +\t\t\tcursor += msg_len; +\t\t\tplen -= msg_len; +\t\t}} +\t}} +\treturn cursor != NULL; +}} +""" + +case_tmpl = """\t\tcase {tlv_msg_enum}: +\t\t\tif (!_fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}cursor, msg_len, {tlv_name}->{tlv_msg_name})) +\t\t\t\treturn false; +\t\t\tbreak; +""" + + +def build_tlv_fromwires(tlv_fields): + fromwires = [] + for field_name, messages in tlv_fields.items(): + fromwires.append(print_tlv_fromwire(field_name, messages)) + return fromwires + + +def build_tlv_towires(tlv_fields): + towires = [] + for field_name, messages in tlv_fields.items(): + towires.append(print_tlv_towire(field_name, messages)) + return towires + + +def print_tlv_towire(tlv_field_name, messages): + fields = "" + for m in messages: + fields += tlv__type_impl_towire_fields.format( + tlv_name=tlv_field_name, + enum=m.enum.name, + name=m.name) + return tlv__type_impl_towire_template.format( + tlv_name=tlv_field_name, + fields=fields) + + +def print_tlv_fromwire(tlv_field_name, messages): + cases = "" + for m in messages: + ctx_arg = 'ctx, ' if m.has_variable_fields else '' + cases += case_tmpl.format(ctx_arg=ctx_arg, + tlv_msg_enum=m.enum.name, + tlv_name=tlv_field_name, + tlv_msg_name=m.name) + return tlv__type_impl_fromwire_template.format( + tlv_name=tlv_field_name, + cases=cases) + def build_tlv_type_struct(name, messages): inner_structs = CCode() @@ -752,7 +986,6 @@ def parse_tlv_file(tlv_field_name): # eg commit_sig,132 tlv_msg = Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments, True) tlv_messages.append(tlv_msg) - messages.append(tlv_msg) tlv_comments = [] tlv_prevfield = None @@ -994,8 +1227,22 @@ def build_tlv_structs(tlv_fields): if options.printwire: decls = [m.print_printwire(options.header) for m in messages + messages_with_option] else: - fromwire_decls = [m.print_fromwire(options.header) for m in messages + messages_with_option] - towire_decls = towire_decls = [m.print_towire(options.header) for m in messages + messages_with_option] + towire_decls = [] + fromwire_decls = [] + + for tlv_field, tlv_messages in tlv_fields.items(): + for m in tlv_messages: + towire_decls.append(m.print_towire(options.header, tlv_field)) + fromwire_decls.append(m.print_fromwire(options.header, tlv_field)) + + if not options.header: + tlv_towires = build_tlv_towires(tlv_fields) + tlv_fromwires = build_tlv_fromwires(tlv_fields) + towire_decls += tlv_towires + fromwire_decls += tlv_fromwires + + towire_decls += [m.print_towire(options.header, '') for m in messages + messages_with_option] + fromwire_decls += [m.print_fromwire(options.header, '') for m in messages + messages_with_option] decls = fromwire_decls + towire_decls print(template.format( From 0e9ad077ed276eb83584bf31748b98b23e784abf Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Wed, 20 Mar 2019 11:49:25 -0700 Subject: [PATCH 08/21] tlv: consolidate basetype parsing clean up basetype parsing code a bit --- tools/generate-wire.py | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index eee5d8ea4c34..4bfbfed46d9a 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -53,6 +53,14 @@ def is_assignable(self): def has_array_helper(self): return self.name in ['u8'] + def base(self): + basetype = self.name + if basetype.startswith('struct '): + basetype = basetype[7:] + elif basetype.startswith('enum '): + basetype = basetype[5:] + return basetype + # Returns base size @staticmethod def _typesize(typename): @@ -171,14 +179,6 @@ def __init__(self, message, name, size, comments, prevname): # Real typename. self.fieldtype = FieldType(size) - def basetype(self): - base = self.fieldtype.name - if base.startswith('struct '): - base = base[7:] - elif base.startswith('enum '): - base = base[5:] - return base - def is_padding(self): return self.name.startswith('pad') @@ -378,7 +378,7 @@ def addField(self, field): if field.is_variable_size(): self.checkLenField(field) self.has_variable_fields = True - elif field.basetype() in varlen_structs or field.optional: + elif field.fieldtype.base() in varlen_structs or field.optional: self.has_variable_fields = True self.fields.append(field) @@ -409,7 +409,7 @@ def print_tlv_fromwire(self, tlv_name): fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] subcalls = CCode() for f in self.fields: - basetype = f.basetype() + basetype = f.fieldtype.base() if f.is_tlv: raise TypeError('Nested TLVs arent allowed!!') elif f.optional: @@ -482,7 +482,7 @@ def print_fromwire(self, is_header, tlv_name): if f.needs_ptr_to_ptr(): ptrs += '*' # If each type is a variable length, we need a ptr to that. - if f.basetype() in varlen_structs: + if f.fieldtype.base() in varlen_structs: ptrs += '*' args.append(', {} {}{}'.format(f.fieldtype.name, ptrs, f.name)) @@ -492,7 +492,7 @@ def print_fromwire(self, is_header, tlv_name): subcalls = CCode() for f in self.fields: - basetype = f.basetype() + basetype = f.fieldtype.base() for c in f.comments: subcalls.append('/*{} */'.format(c)) @@ -591,12 +591,7 @@ def print_tlv_towire(self, tlv_name): subcalls = CCode() for f in self.fields: - basetype = f.fieldtype.name - if basetype.startswith('struct '): - basetype = basetype[7:] - elif basetype.startswith('enum '): - basetype = basetype[5:] - + basetype = f.fieldtype.base() for c in f.comments: subcalls.append('/*{} */'.format(c)) @@ -632,7 +627,7 @@ def print_towire(self, is_header, tlv_name): args.append(', const struct _{} *{}'.format(f.name, f.name)) elif f.is_assignable(): args.append(', {} {}'.format(f.fieldtype.name, f.name)) - elif f.is_variable_size() and f.basetype() in varlen_structs: + elif f.is_variable_size() and f.fieldtype.base() in varlen_structs: args.append(', const {} **{}'.format(f.fieldtype.name, f.name)) else: args.append(', const {} *{}'.format(f.fieldtype.name, f.name)) @@ -651,11 +646,7 @@ def print_towire(self, is_header, tlv_name): subcalls = CCode() for f in self.fields: - basetype = f.fieldtype.name - if basetype.startswith('struct '): - basetype = basetype[7:] - elif basetype.startswith('enum '): - basetype = basetype[5:] + basetype = f.fieldtype.base() for c in f.comments: subcalls.append('/*{} */'.format(c)) @@ -735,7 +726,7 @@ def print_printwire(self, is_header): subcalls = CCode() for f in self.fields: - basetype = f.basetype() + basetype = f.fieldtype.base() for c in f.comments: subcalls.append('/*{} */'.format(c)) From ea9c800e73f68dcdd58d467f0f97a5eaecf8d599 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:25:19 -0700 Subject: [PATCH 09/21] tlv: calculate sizeof by measuring message length much better than statically calculating the sizeof --- tools/generate-wire.py | 110 +++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 49 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 4bfbfed46d9a..a0744328a1bf 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -235,15 +235,12 @@ def _guess_type(message, fieldname, base_size): fromwire_tlv_impl_templ = """static bool _fromwire_{tlv_name}_{name}({ctx}{args}) {{ -\tsize_t start_len, plen; +\tsize_t start_len = *plen; {fields} -\tconst u8 *cursor = p; -\tplen = tal_count(p); -\tif (plen < len) +\tif (start_len < len) \t\treturn false; -\tstart_len = plen; {subcalls} -\treturn cursor != NULL && (start_len - plen == len); +\treturn cursor != NULL && (start_len - *plen == len); }} """ @@ -382,22 +379,23 @@ def addField(self, field): self.has_variable_fields = True self.fields.append(field) - def print_fromwire_array(self, ctx, subcalls, basetype, f, name, num_elems): + def print_fromwire_array(self, ctx, subcalls, basetype, f, name, num_elems, is_tlv=False): + p_ref = '' if is_tlv else '&' if f.has_array_helper(): - subcalls.append('fromwire_{}_array(&cursor, &plen, {}, {});' - .format(basetype, name, num_elems)) + subcalls.append('fromwire_{}_array(&cursor, {}plen, {}, {});' + .format(basetype, p_ref, name, num_elems)) else: subcalls.append('for (size_t i = 0; i < {}; i++)' .format(num_elems)) if f.fieldtype.is_assignable(): - subcalls.append('({})[i] = fromwire_{}(&cursor, &plen);' - .format(name, basetype)) + subcalls.append('({})[i] = fromwire_{}(&cursor, {}plen);' + .format(name, basetype, p_ref)) elif basetype in varlen_structs: - subcalls.append('({})[i] = fromwire_{}({}, &cursor, &plen);' - .format(name, basetype, ctx)) + subcalls.append('({})[i] = fromwire_{}({}, &cursor, {}plen);' + .format(name, basetype, ctx, p_ref)) else: - subcalls.append('fromwire_{}(&cursor, &plen, {} + i);' - .format(basetype, name)) + subcalls.append('fromwire_{}(&cursor, {}plen, {} + i);' + .format(basetype, p_ref, name)) def print_tlv_fromwire(self, tlv_name): """ prints fromwire function definition for a TLV message. @@ -405,7 +403,7 @@ def print_tlv_fromwire(self, tlv_name): to populate, instead of fields, as well as a length to read in """ ctx_arg = 'const tal_t *ctx, ' if self.has_variable_fields else '' - args = 'const void *p, const u16 len, struct _tlv_msg_{name} *{name}'.format(name=self.name) + args = 'const u8 *cursor, size_t *plen, const u16 len, struct _tlv_msg_{name} *{name}'.format(name=self.name) fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] subcalls = CCode() for f in self.fields: @@ -419,12 +417,12 @@ def print_tlv_fromwire(self, tlv_name): subcalls.append('/*{} */'.format(c)) if f.is_padding(): - subcalls.append('fromwire_pad(&cursor, &plen, {});' + subcalls.append('fromwire_pad(&cursor, plen, {});' .format(f.num_elems)) elif f.is_array(): name = '*{}->{}'.format(self.name, f.name) self.print_fromwire_array('ctx', subcalls, basetype, f, name, - f.num_elems) + f.num_elems, is_tlv=True) elif f.is_variable_size(): subcalls.append("// 2nd case {name}".format(name=f.name)) typename = f.fieldtype.name @@ -437,16 +435,16 @@ def print_tlv_fromwire(self, tlv_name): name = '{}->{}'.format(self.name, f.name) # Allocate these off the array itself, if they need alloc. self.print_fromwire_array('*' + f.name, subcalls, basetype, f, - name, f.lenvar) + name, f.lenvar, is_tlv=True) else: if f.is_assignable(): if f.is_len_var: - s = '{} = fromwire_{}(&cursor, &plen);'.format(f.name, basetype) + s = '{} = fromwire_{}(&cursor, plen);'.format(f.name, basetype) else: - s = '{}->{} = fromwire_{}(&cursor, &plen);'.format( + s = '{}->{} = fromwire_{}(&cursor, plen);'.format( self.name, f.name, basetype) else: - s = 'fromwire_{}(&cursor, &plen, *{}->{});'.format( + s = 'fromwire_{}(&cursor, plen, *{}->{});'.format( basetype, self.name, f.name) subcalls.append(s) @@ -506,7 +504,7 @@ def print_fromwire(self, is_header, tlv_name): elif f.is_tlv: if not f.is_variable_size(): raise TypeError('TLV {} not variable size'.format(f.name)) - subcalls.append('if (!fromwire__{tlv_name}(ctx, &cursor, &{tlv_len}, {tlv_name}))' + subcalls.append('if (!fromwire__{tlv_name}(ctx, &cursor, &plen, &{tlv_len}, {tlv_name}))' .format(tlv_name=f.name, tlv_len=f.lenvar)) subcalls.append('return false;') elif f.is_variable_size(): @@ -585,7 +583,7 @@ def print_tlv_towire(self, tlv_name): elif f.optional: raise TypeError("Optional fields on TLV messages not currently supported. {}->{}".format(tlv_name, f.name)) if f.is_len_var: - field_decls.append('\t{0} {1} = tal_count(&{2}->{3});'.format( + field_decls.append('\t{0} {1} = tal_count({2}->{3});'.format( f.fieldtype.name, f.name, self.name, f.lenvar_for.name )) @@ -611,6 +609,9 @@ def print_tlv_towire(self, tlv_name): field_decls='\n'.join(field_decls), subcalls=str(subcalls)) + def find_tlv_lenvar_field(self, tlv_name): + return [f for f in self.fields if f.is_len_var and f.lenvar_for.is_tlv and f.lenvar_for.name == tlv_name][0] + def print_towire(self, is_header, tlv_name): if self.is_tlv: if is_header: @@ -636,9 +637,8 @@ def print_towire(self, is_header, tlv_name): for f in self.fields: if f.is_len_var: if f.lenvar_for.is_tlv: - field_decls.append('\t{0} {1} = sizeof({2});'.format( - f.fieldtype.name, f.name, f.lenvar_for.name - )) + # used below... + field_decls.append('\t{0} {1};'.format(f.fieldtype.name, f.name)) else: field_decls.append('\t{0} {1} = tal_count({2});'.format( f.fieldtype.name, f.name, f.lenvar_for.name @@ -656,11 +656,21 @@ def print_towire(self, is_header, tlv_name): .format(f.num_elems)) elif f.is_array(): self.print_towire_array(subcalls, basetype, f, f.num_elems) + elif f.is_len_var and f.lenvar_for.is_tlv: + continue # taken care of below elif f.is_tlv: if not f.is_variable_size(): - raise TypeError('TLV {} not variable size'.format(f.name)) - subcalls.append('towire__{tlv_name}(&p, {tlv_name});'.format( - tlv_name=f.name)) + raise ValueError('TLV {} not variable size'.format(f.name)) + lenvar_field = self.find_tlv_lenvar_field(f.name) + subcalls.append('/* ~~build TLV for {} ~~*/'.format(f.name)) + subcalls.append("u8 *{tlv_name}_buffer = tal_arr(ctx, u8, 0);\n" + "towire__{tlv_name}(ctx, &{tlv_name}_buffer, {tlv_name});\n" + "{lenvar_field} = tal_count({tlv_name}_buffer);\n" + "towire_{lenvar_fieldtype}(&p, {lenvar_field});\n" + "towire_u8_array(&p, {tlv_name}_buffer, {lenvar_field});\n".format( + tlv_name=f.name, + lenvar_field=lenvar_field.name, + lenvar_fieldtype=lenvar_field.fieldtype.name)) elif f.is_variable_size(): self.print_towire_array(subcalls, basetype, f, f.lenvar) else: @@ -821,43 +831,47 @@ def print_struct(self): """ tlv__type_impl_towire_fields = """\tif ({tlv_name}->{name}) {{ +\t\ttlv_msg = tal_arr(ctx, u8, 0); \t\ttowire_u16(p, {enum}); -\t\ttowire_u16(p, sizeof(*{tlv_name}->{name})); -\t\t_towire_{tlv_name}_{name}(p, {tlv_name}->{name}); +\t\t_towire_{tlv_name}_{name}(&tlv_msg, {tlv_name}->{name}); +\t\tmsg_len = tal_count(tlv_msg); +\t\ttowire_u16(p, msg_len); +\t\ttowire_u8_array(p, tlv_msg, msg_len); \t}} """ -tlv__type_impl_towire_template = """static void towire__{tlv_name}(u8 **p, const struct _{tlv_name} *{tlv_name}) {{ +tlv__type_impl_towire_template = """static void towire__{tlv_name}(const tal_t *ctx, u8 **p, const struct _{tlv_name} *{tlv_name}) {{ +\tu16 msg_len; +\tu8 *tlv_msg; {fields}}} """ -tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, const u16 *len, struct _{tlv_name} *{tlv_name}) {{ +tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len, struct _{tlv_name} *{tlv_name}) {{ \tu16 msg_type, msg_len; -\tconst u8 *cursor = *p; -\tsize_t plen = tal_count(p); -\tif (plen != *len) +\tif (*plen < *len) \t\treturn false; -\twhile (cursor && plen) {{ -\t\tmsg_type = fromwire_u16(&cursor, &plen); -\t\tmsg_len = fromwire_u16(&cursor, &plen); -\t\tif (plen < msg_len) {{ -\t\t\tfromwire_fail(&cursor, &plen); +\twhile (*plen) {{ +\t\tmsg_type = fromwire_u16(p, plen); +\t\tmsg_len = fromwire_u16(p, plen); +\t\tif (*plen < msg_len) {{ +\t\t\tfromwire_fail(p, plen); \t\t\tbreak; \t\t}} \t\tswitch((enum {tlv_name}_type)msg_type) {{ {cases}\t\tdefault: \t\t\t// FIXME: print a warning / message? -\t\t\tcursor += msg_len; +\t\t\t*p+= msg_len; \t\t\tplen -= msg_len; \t\t}} \t}} -\treturn cursor != NULL; +\treturn *p != NULL; }} """ case_tmpl = """\t\tcase {tlv_msg_enum}: -\t\t\tif (!_fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}cursor, msg_len, {tlv_name}->{tlv_msg_name})) +\t\t\t{tlv_name}->{tlv_msg_name} = tal(ctx, struct _tlv_msg_{tlv_msg_name}); +\t\t\tif (!_fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}*p, plen, msg_len, {tlv_name}->{tlv_msg_name})) \t\t\t\treturn false; \t\t\tbreak; """ @@ -1227,10 +1241,8 @@ def build_tlv_structs(tlv_fields): fromwire_decls.append(m.print_fromwire(options.header, tlv_field)) if not options.header: - tlv_towires = build_tlv_towires(tlv_fields) - tlv_fromwires = build_tlv_fromwires(tlv_fields) - towire_decls += tlv_towires - fromwire_decls += tlv_fromwires + towire_decls += build_tlv_towires(tlv_fields) + fromwire_decls += build_tlv_fromwires(tlv_fields) towire_decls += [m.print_towire(options.header, '') for m in messages + messages_with_option] fromwire_decls += [m.print_fromwire(options.header, '') for m in messages + messages_with_option] From a9dee94d6f82278557f9fb5264807498b7f77713 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:26:25 -0700 Subject: [PATCH 10/21] tlv: remove leading '_' from things Suggested-By: @rustyrussell --- tools/generate-wire.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index a0744328a1bf..bd1d177ff29d 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -232,7 +232,7 @@ def _guess_type(message, fieldname, base_size): }} """ -fromwire_tlv_impl_templ = """static bool _fromwire_{tlv_name}_{name}({ctx}{args}) +fromwire_tlv_impl_templ = """static bool fromwire_{tlv_name}_{name}({ctx}{args}) {{ \tsize_t start_len = *plen; @@ -403,7 +403,7 @@ def print_tlv_fromwire(self, tlv_name): to populate, instead of fields, as well as a length to read in """ ctx_arg = 'const tal_t *ctx, ' if self.has_variable_fields else '' - args = 'const u8 *cursor, size_t *plen, const u16 len, struct _tlv_msg_{name} *{name}'.format(name=self.name) + args = 'const u8 *cursor, size_t *plen, const u16 len, struct tlv_msg_{name} *{name}'.format(name=self.name) fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] subcalls = CCode() for f in self.fields: @@ -473,7 +473,7 @@ def print_fromwire(self, is_header, tlv_name): elif f.is_array(): args.append(', {} {}[{}]'.format(f.fieldtype.name, f.name, f.num_elems)) elif f.is_tlv: - args.append(', struct _{} *{}'.format(f.name, f.name)) + args.append(', struct {} *{}'.format(f.name, f.name)) else: ptrs = '*' # If we're handing a variable array, we need a ptr-to-ptr. @@ -625,7 +625,7 @@ def print_towire(self, is_header, tlv_name): if f.is_array(): args.append(', const {} {}[{}]'.format(f.fieldtype.name, f.name, f.num_elems)) elif f.is_tlv: - args.append(', const struct _{} *{}'.format(f.name, f.name)) + args.append(', const struct {} *{}'.format(f.name, f.name)) elif f.is_assignable(): args.append(', {} {}'.format(f.fieldtype.name, f.name)) elif f.is_variable_size() and f.fieldtype.base() in varlen_structs: @@ -811,7 +811,7 @@ def print_struct(self): fields=str(fmt_fields)) -tlv_message_towire_stub = """static void _towire_{tlv_name}_{name}(u8 **p, struct _tlv_msg_{name} *{name}) {{ +tlv_message_towire_stub = """static void towire_{tlv_name}_{name}(u8 **p, struct tlv_msg_{name} *{name}) {{ {field_decls} {subcalls} }} @@ -819,13 +819,13 @@ def print_struct(self): tlv_msg_struct_template = """ -struct _tlv_msg_{msg_name} {{ +struct tlv_msg_{msg_name} {{ {fields} }}; """ tlv_struct_template = """ -struct _{tlv_name} {{ +struct {tlv_name} {{ {msg_type_structs} }}; """ @@ -833,20 +833,20 @@ def print_struct(self): tlv__type_impl_towire_fields = """\tif ({tlv_name}->{name}) {{ \t\ttlv_msg = tal_arr(ctx, u8, 0); \t\ttowire_u16(p, {enum}); -\t\t_towire_{tlv_name}_{name}(&tlv_msg, {tlv_name}->{name}); +\t\ttowire_{tlv_name}_{name}(&tlv_msg, {tlv_name}->{name}); \t\tmsg_len = tal_count(tlv_msg); \t\ttowire_u16(p, msg_len); \t\ttowire_u8_array(p, tlv_msg, msg_len); \t}} """ -tlv__type_impl_towire_template = """static void towire__{tlv_name}(const tal_t *ctx, u8 **p, const struct _{tlv_name} *{tlv_name}) {{ +tlv__type_impl_towire_template = """static void towire__{tlv_name}(const tal_t *ctx, u8 **p, const struct {tlv_name} *{tlv_name}) {{ \tu16 msg_len; \tu8 *tlv_msg; {fields}}} """ -tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len, struct _{tlv_name} *{tlv_name}) {{ +tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len, struct {tlv_name} *{tlv_name}) {{ \tu16 msg_type, msg_len; \tif (*plen < *len) \t\treturn false; @@ -870,8 +870,8 @@ def print_struct(self): """ case_tmpl = """\t\tcase {tlv_msg_enum}: -\t\t\t{tlv_name}->{tlv_msg_name} = tal(ctx, struct _tlv_msg_{tlv_msg_name}); -\t\t\tif (!_fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}*p, plen, msg_len, {tlv_name}->{tlv_msg_name})) +\t\t\t{tlv_name}->{tlv_msg_name} = tal(ctx, struct tlv_msg_{tlv_msg_name}); +\t\t\tif (!fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}*p, plen, msg_len, {tlv_name}->{tlv_msg_name})) \t\t\t\treturn false; \t\t\tbreak; """ @@ -919,7 +919,7 @@ def print_tlv_fromwire(tlv_field_name, messages): def build_tlv_type_struct(name, messages): inner_structs = CCode() for m in messages: - inner_structs.append('struct _tlv_msg_{} *{};'.format(m.name, m.name)) + inner_structs.append('struct tlv_msg_{} *{};'.format(m.name, m.name)) return tlv_struct_template.format( tlv_name=name, msg_type_structs=str(inner_structs)) From 2c698de019378d4d993f739f297ee6d6d3567a94 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Mon, 25 Mar 2019 18:33:29 -0700 Subject: [PATCH 11/21] tlv: make message sizes u8 not u16 Suggested-By: @rustyrussell --- tools/generate-wire.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index bd1d177ff29d..7a6ff8fc2381 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -832,28 +832,28 @@ def print_struct(self): tlv__type_impl_towire_fields = """\tif ({tlv_name}->{name}) {{ \t\ttlv_msg = tal_arr(ctx, u8, 0); -\t\ttowire_u16(p, {enum}); +\t\ttowire_u8(p, {enum}); \t\ttowire_{tlv_name}_{name}(&tlv_msg, {tlv_name}->{name}); \t\tmsg_len = tal_count(tlv_msg); -\t\ttowire_u16(p, msg_len); +\t\ttowire_u8(p, msg_len); \t\ttowire_u8_array(p, tlv_msg, msg_len); \t}} """ tlv__type_impl_towire_template = """static void towire__{tlv_name}(const tal_t *ctx, u8 **p, const struct {tlv_name} *{tlv_name}) {{ -\tu16 msg_len; +\tu8 msg_len; \tu8 *tlv_msg; {fields}}} """ tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len, struct {tlv_name} *{tlv_name}) {{ -\tu16 msg_type, msg_len; +\tu8 msg_type, msg_len; \tif (*plen < *len) \t\treturn false; \twhile (*plen) {{ -\t\tmsg_type = fromwire_u16(p, plen); -\t\tmsg_len = fromwire_u16(p, plen); +\t\tmsg_type = fromwire_u8(p, plen); +\t\tmsg_len = fromwire_u8(p, plen); \t\tif (*plen < msg_len) {{ \t\t\tfromwire_fail(p, plen); \t\t\tbreak; From d8171e12e481fe0d6a1e1e9835d873a948ece255 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:27:28 -0700 Subject: [PATCH 12/21] tlv: allocate tlv structs from within let's let the fromwire__tlv methods allocate the tlv-objects and return them. we also want to initialize all of their underlying messages to NULL, and fail if we discover a duplicate mesage type. if parsing fails, instead of returning a struct we return NULL. Suggested-By: @rustyrussell --- tools/generate-wire.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 7a6ff8fc2381..ead9ffcc5b5e 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -504,8 +504,9 @@ def print_fromwire(self, is_header, tlv_name): elif f.is_tlv: if not f.is_variable_size(): raise TypeError('TLV {} not variable size'.format(f.name)) - subcalls.append('if (!fromwire__{tlv_name}(ctx, &cursor, &plen, &{tlv_len}, {tlv_name}))' + subcalls.append('{tlv_name} = fromwire__{tlv_name}(ctx, &cursor, &plen, &{tlv_len});' .format(tlv_name=f.name, tlv_len=f.lenvar)) + subcalls.append('if (!{tlv_name})'.format(tlv_name=f.name)) subcalls.append('return false;') elif f.is_variable_size(): subcalls.append("//2nd case {name}".format(name=f.name)) @@ -846,10 +847,12 @@ def print_struct(self): {fields}}} """ -tlv__type_impl_fromwire_template = """static bool fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len, struct {tlv_name} *{tlv_name}) {{ +tlv__type_impl_fromwire_template = """static struct {tlv_name} *fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len) {{ \tu8 msg_type, msg_len; \tif (*plen < *len) -\t\treturn false; +\t\treturn NULL; + +\tstruct {tlv_name} *{tlv_name} = talz(ctx, struct {tlv_name}); \twhile (*plen) {{ \t\tmsg_type = fromwire_u8(p, plen); @@ -865,14 +868,25 @@ def print_struct(self): \t\t\tplen -= msg_len; \t\t}} \t}} -\treturn *p != NULL; +\tif (!*p) {{ +\t\ttal_free({tlv_name}); +\t\treturn NULL; +\t}} +\treturn {tlv_name}; }} """ case_tmpl = """\t\tcase {tlv_msg_enum}: -\t\t\t{tlv_name}->{tlv_msg_name} = tal(ctx, struct tlv_msg_{tlv_msg_name}); -\t\t\tif (!fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}*p, plen, msg_len, {tlv_name}->{tlv_msg_name})) -\t\t\t\treturn false; +\t\t\tif ({tlv_name}->{tlv_msg_name} != NULL) {{ +\t\t\t\tfromwire_fail(p, plen); +\t\t\t\ttal_free({tlv_name}); +\t\t\t\treturn NULL; +\t\t\t}} +\t\t\t{tlv_name}->{tlv_msg_name} = tal({tlv_name}, struct tlv_msg_{tlv_msg_name}); +\t\t\tif (!fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}*p, plen, msg_len, {tlv_name}->{tlv_msg_name})) {{ +\t\t\t\ttal_free({tlv_name}); +\t\t\t\treturn NULL; +\t\t\t}} \t\t\tbreak; """ @@ -906,7 +920,7 @@ def print_tlv_towire(tlv_field_name, messages): def print_tlv_fromwire(tlv_field_name, messages): cases = "" for m in messages: - ctx_arg = 'ctx, ' if m.has_variable_fields else '' + ctx_arg = tlv_field_name + ', ' if m.has_variable_fields else '' cases += case_tmpl.format(ctx_arg=ctx_arg, tlv_msg_enum=m.enum.name, tlv_name=tlv_field_name, From e161106c63130f28325ac424acfc2158539c5463 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Sun, 24 Mar 2019 16:23:34 -0700 Subject: [PATCH 13/21] tlv: it's ok to be odd fail if a message type is even and it's not included. otherwise, continue with the next message type. --- tools/generate-wire.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index ead9ffcc5b5e..870272351d41 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -863,9 +863,13 @@ def print_struct(self): \t\t}} \t\tswitch((enum {tlv_name}_type)msg_type) {{ {cases}\t\tdefault: -\t\t\t// FIXME: print a warning / message? -\t\t\t*p+= msg_len; -\t\t\tplen -= msg_len; +\t\t\tif (msg_type % 2 == 0) {{ // it's ok to be odd +\t\t\t\tfromwire_fail(p, plen); +\t\t\t\ttal_free({tlv_name}); +\t\t\t\treturn NULL; +\t\t\t}} +\t\t\t*p += msg_len; +\t\t\t*plen -= msg_len; \t\t}} \t}} \tif (!*p) {{ From c06f2a7de8060802e4eb2cfe4cabaf8df115b89f Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:28:08 -0700 Subject: [PATCH 14/21] tlv: don't crash when you fail to parse the TLV passing back a null TLV was crashing here, because we tried to dereference a null pointer. instead, we put it into a temporary struct that we can check for NULL-ness, before assigning to the passed in pointer. --- tools/generate-wire.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 870272351d41..9ecad810f306 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -504,10 +504,11 @@ def print_fromwire(self, is_header, tlv_name): elif f.is_tlv: if not f.is_variable_size(): raise TypeError('TLV {} not variable size'.format(f.name)) - subcalls.append('{tlv_name} = fromwire__{tlv_name}(ctx, &cursor, &plen, &{tlv_len});' + subcalls.append('struct {tlv_name} *_tlv = fromwire__{tlv_name}(ctx, &cursor, &plen, &{tlv_len});' .format(tlv_name=f.name, tlv_len=f.lenvar)) - subcalls.append('if (!{tlv_name})'.format(tlv_name=f.name)) + subcalls.append('if (!_tlv)') subcalls.append('return false;') + subcalls.append('*{tlv_name} = *_tlv;'.format(tlv_name=f.name)) elif f.is_variable_size(): subcalls.append("//2nd case {name}".format(name=f.name)) typename = f.fieldtype.name From 2302e43eee6445467bb64d4ccd2d07cfc2fc4b8e Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Sun, 24 Mar 2019 19:35:25 -0700 Subject: [PATCH 15/21] tlv: fail if parsed length doesn't match packet length --- tools/generate-wire.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 9ecad810f306..74e436509c92 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -850,6 +850,7 @@ def print_struct(self): tlv__type_impl_fromwire_template = """static struct {tlv_name} *fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len) {{ \tu8 msg_type, msg_len; +\tsize_t start_len = *plen; \tif (*plen < *len) \t\treturn NULL; @@ -873,7 +874,7 @@ def print_struct(self): \t\t\t*plen -= msg_len; \t\t}} \t}} -\tif (!*p) {{ +\tif (!*p || start_len - *plen != *len) {{ \t\ttal_free({tlv_name}); \t\treturn NULL; \t}} From 3f711af1cfc8ad8303bbdc6d7a867736d5d6aff8 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Thu, 28 Mar 2019 14:28:56 -0700 Subject: [PATCH 16/21] tlv: fixup parsing for multi-message tlv's need to pass in a pointer to the array so that when we advance the array in the subcalls, it advances in the parent. oops --- tools/generate-wire.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 74e436509c92..0616e35b4299 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -382,20 +382,20 @@ def addField(self, field): def print_fromwire_array(self, ctx, subcalls, basetype, f, name, num_elems, is_tlv=False): p_ref = '' if is_tlv else '&' if f.has_array_helper(): - subcalls.append('fromwire_{}_array(&cursor, {}plen, {}, {});' - .format(basetype, p_ref, name, num_elems)) + subcalls.append('fromwire_{}_array({}cursor, {}plen, {}, {});' + .format(basetype, p_ref, p_ref, name, num_elems)) else: subcalls.append('for (size_t i = 0; i < {}; i++)' .format(num_elems)) if f.fieldtype.is_assignable(): - subcalls.append('({})[i] = fromwire_{}(&cursor, {}plen);' - .format(name, basetype, p_ref)) + subcalls.append('({})[i] = fromwire_{}({}cursor, {}plen);' + .format(name, basetype, p_ref, p_ref)) elif basetype in varlen_structs: - subcalls.append('({})[i] = fromwire_{}({}, &cursor, {}plen);' - .format(name, basetype, ctx, p_ref)) + subcalls.append('({})[i] = fromwire_{}({}, {}cursor, {}plen);' + .format(name, basetype, ctx, p_ref, p_ref)) else: - subcalls.append('fromwire_{}(&cursor, {}plen, {} + i);' - .format(basetype, p_ref, name)) + subcalls.append('fromwire_{}({}cursor, {}plen, {} + i);' + .format(basetype, p_ref, p_ref, name)) def print_tlv_fromwire(self, tlv_name): """ prints fromwire function definition for a TLV message. @@ -403,7 +403,7 @@ def print_tlv_fromwire(self, tlv_name): to populate, instead of fields, as well as a length to read in """ ctx_arg = 'const tal_t *ctx, ' if self.has_variable_fields else '' - args = 'const u8 *cursor, size_t *plen, const u16 len, struct tlv_msg_{name} *{name}'.format(name=self.name) + args = 'const u8 **cursor, size_t *plen, const u16 len, struct tlv_msg_{name} *{name}'.format(name=self.name) fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] subcalls = CCode() for f in self.fields: @@ -417,7 +417,7 @@ def print_tlv_fromwire(self, tlv_name): subcalls.append('/*{} */'.format(c)) if f.is_padding(): - subcalls.append('fromwire_pad(&cursor, plen, {});' + subcalls.append('fromwire_pad(cursor, plen, {});' .format(f.num_elems)) elif f.is_array(): name = '*{}->{}'.format(self.name, f.name) @@ -439,12 +439,12 @@ def print_tlv_fromwire(self, tlv_name): else: if f.is_assignable(): if f.is_len_var: - s = '{} = fromwire_{}(&cursor, plen);'.format(f.name, basetype) + s = '{} = fromwire_{}(cursor, plen);'.format(f.name, basetype) else: - s = '{}->{} = fromwire_{}(&cursor, plen);'.format( + s = '{}->{} = fromwire_{}(cursor, plen);'.format( self.name, f.name, basetype) else: - s = 'fromwire_{}(&cursor, plen, *{}->{});'.format( + s = 'fromwire_{}(cursor, plen, *{}->{});'.format( basetype, self.name, f.name) subcalls.append(s) @@ -889,7 +889,7 @@ def print_struct(self): \t\t\t\treturn NULL; \t\t\t}} \t\t\t{tlv_name}->{tlv_msg_name} = tal({tlv_name}, struct tlv_msg_{tlv_msg_name}); -\t\t\tif (!fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}*p, plen, msg_len, {tlv_name}->{tlv_msg_name})) {{ +\t\t\tif (!fromwire_{tlv_name}_{tlv_msg_name}({ctx_arg}p, plen, msg_len, {tlv_name}->{tlv_msg_name})) {{ \t\t\t\ttal_free({tlv_name}); \t\t\t\treturn NULL; \t\t\t}} From c8bc0a099a578f772bb51e4e9e6da9fd7fd3577d Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Mon, 25 Mar 2019 17:48:01 -0700 Subject: [PATCH 17/21] tlv: free intermediate messages when they're created otherwise they'll get cleaned up when the message is free'd. it's nbd either way, but this seems tighter. --- tools/generate-wire.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 0616e35b4299..e40b52a9241a 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -839,6 +839,7 @@ def print_struct(self): \t\tmsg_len = tal_count(tlv_msg); \t\ttowire_u8(p, msg_len); \t\ttowire_u8_array(p, tlv_msg, msg_len); +\t\ttal_free(tlv_msg); \t}} """ From a889c6a79317d08156dc7b6e36977de7d26eb966 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Fri, 8 Mar 2019 17:50:52 -0800 Subject: [PATCH 18/21] tlv: fixup FIXME -- remove comments + use includes include includes for TLV _csv files --- tools/generate-wire.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index e40b52a9241a..922db9a6ba4f 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -126,7 +126,7 @@ def _typesize(typename): # arraysize := length '*' type # length := lenvar | number class Field(object): - def __init__(self, message, name, size, comments, prevname): + def __init__(self, message, name, size, comments, prevname, includes): self.message = message self.comments = comments self.name = name @@ -141,8 +141,8 @@ def __init__(self, message, name, size, comments, prevname): self.is_tlv = True self.name = name[:-1] if self.name not in tlv_fields: - # FIXME: use the rest of this - tlv_includes, tlv_messages, tlv_comments = parse_tlv_file(self.name) + tlv_includes, tlv_messages = parse_tlv_file(self.name) + includes += tlv_includes tlv_fields[self.name] = tlv_messages # ? means optional field (not supported for arrays) @@ -1029,14 +1029,14 @@ def parse_tlv_file(tlv_field_name): else: raise ValueError('Line {} malformed'.format(line.rstrip())) - f = Field(m.name, parts[2], parts[3], tlv_comments, tlv_prevfield) + f = Field(m.name, parts[2], parts[3], tlv_comments, tlv_prevfield, includes) m.addField(f) # If it used prevfield as lenvar, keep that for next # time (multiple fields can use the same lenvar). if not f.lenvar: tlv_prevfield = parts[2] tlv_comments = [] - return tlv_includes, tlv_messages, tlv_comments + return tlv_includes, tlv_messages parser = argparse.ArgumentParser(description='Generate C from CSV') @@ -1092,7 +1092,7 @@ def parse_tlv_file(tlv_field_name): else: raise ValueError('Line {} malformed'.format(line.rstrip())) - f = Field(m.name, parts[2], parts[3], comments, prevfield) + f = Field(m.name, parts[2], parts[3], comments, prevfield, includes) m.addField(f) # If it used prevfield as lenvar, keep that for next # time (multiple fields can use the same lenvar). From 73e0518be075c8c2eb7bdc272d251c259c2cda21 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Tue, 2 Apr 2019 19:29:48 -0700 Subject: [PATCH 19/21] tlv: adapt to work with new output format Updated to match what the CSV generator in the RFC repo actually outputs, see https://github.com/lightningnetwork/lightning-rfc/pull/597 --- tools/generate-wire.py | 89 ++++++++---------------------------------- 1 file changed, 16 insertions(+), 73 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 922db9a6ba4f..4e2ddbe1a571 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -136,14 +136,10 @@ def __init__(self, message, name, size, comments, prevname, includes): self.optional = False self.is_tlv = False - # field name appended with '+' means this field contains a tlv - if name.endswith('+'): + if name.endswith('_tlv'): self.is_tlv = True - self.name = name[:-1] if self.name not in tlv_fields: - tlv_includes, tlv_messages = parse_tlv_file(self.name) - includes += tlv_includes - tlv_fields[self.name] = tlv_messages + tlv_fields[self.name] = [] # ? means optional field (not supported for arrays) if size.startswith('?'): @@ -977,68 +973,6 @@ def find_message_with_option(messages, optional_messages, name, option): return m -def get_directory_prefix(): - # FIXME: use prefix of filename - return "wire/" - - -def get_tlv_filename(field_name): - return 'gen_{}_csv'.format(field_name) - - -def parse_tlv_file(tlv_field_name): - tlv_includes = [] - tlv_messages = [] - tlv_comments = [] - tlv_prevfield = None - with open(get_directory_prefix() + get_tlv_filename(tlv_field_name)) as f: - for line in f: - # #include gets inserted into header - if line.startswith('#include '): - tlv_includes.append(line) - continue - - by_comments = line.rstrip().split('#') - - # Emit a comment if they included one - if by_comments[1:]: - tlv_comments.append(' '.join(by_comments[1:])) - - parts = by_comments[0].split(',') - if parts == ['']: - continue - - if len(parts) == 2: - # eg commit_sig,132 - tlv_msg = Message(parts[0], Enumtype("TLV_" + parts[0].upper(), parts[1]), tlv_comments, True) - tlv_messages.append(tlv_msg) - - tlv_comments = [] - tlv_prevfield = None - else: - if len(parts) == 4: - # eg commit_sig,0,channel-id,8 OR - # commit_sig,0,channel-id,u64 - m = find_message(tlv_messages, parts[0]) - if m is None: - raise ValueError('Unknown message {}'.format(parts[0])) - elif len(parts) == 5: - # eg. - # channel_reestablish,48,your_last_per_commitment_secret,32,option209 - m = find_message_with_option(tlv_messages, messages_with_option, parts[0], parts[4]) - else: - raise ValueError('Line {} malformed'.format(line.rstrip())) - - f = Field(m.name, parts[2], parts[3], tlv_comments, tlv_prevfield, includes) - m.addField(f) - # If it used prevfield as lenvar, keep that for next - # time (multiple fields can use the same lenvar). - if not f.lenvar: - tlv_prevfield = parts[2] - tlv_comments = [] - return tlv_includes, tlv_messages - - parser = argparse.ArgumentParser(description='Generate C from CSV') parser.add_argument('--header', action='store_true', help="Create wire header") parser.add_argument('--bolt', action='store_true', help="Generate wire-format for BOLT") @@ -1073,9 +1007,18 @@ def parse_tlv_file(tlv_field_name): if parts == ['']: continue - if len(parts) == 2: - # eg commit_sig,132 - messages.append(Message(parts[0], Enumtype("WIRE_" + parts[0].upper(), parts[1]), comments, False)) + is_tlv_msg = len(parts) == 3 + if len(parts) == 2 or is_tlv_msg: + # eg: commit_sig,132,(_tlv) + message = Message(parts[0], + Enumtype("WIRE_" + parts[0].upper(), parts[1]), + comments, + is_tlv_msg) + + messages.append(message) + if is_tlv_msg: + tlv_fields[parts[2]].append(message) + comments = [] prevfield = None else: @@ -1265,8 +1208,8 @@ def build_tlv_structs(tlv_fields): towire_decls += build_tlv_towires(tlv_fields) fromwire_decls += build_tlv_fromwires(tlv_fields) - towire_decls += [m.print_towire(options.header, '') for m in messages + messages_with_option] - fromwire_decls += [m.print_fromwire(options.header, '') for m in messages + messages_with_option] + towire_decls += [m.print_towire(options.header, '') for m in toplevel_messages + messages_with_option] + fromwire_decls += [m.print_fromwire(options.header, '') for m in toplevel_messages + messages_with_option] decls = fromwire_decls + towire_decls print(template.format( From 2cd37fad5b21751128ff9f549331af9f4761c2be Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Tue, 2 Apr 2019 17:18:06 -0700 Subject: [PATCH 20/21] wire: add var_int parsing functions so we can put and pull bitcoin 'var_int' length types from the wire. for more info on variable integers, see http://learnmeabitcoin.com/glossary/varint --- wire/fromwire.c | 21 +++++++++++++++++++++ wire/towire.c | 16 ++++++++++++++++ wire/wire.h | 2 ++ 3 files changed, 39 insertions(+) diff --git a/wire/fromwire.c b/wire/fromwire.c index b7d5a6ad1ba7..edcda6ae3506 100644 --- a/wire/fromwire.c +++ b/wire/fromwire.c @@ -101,6 +101,27 @@ bool fromwire_bool(const u8 **cursor, size_t *max) return ret; } +u64 fromwire_var_int(const u8 **cursor, size_t *max) +{ + if (*max < 1) { + fromwire_fail(cursor, max); + return 0; + } + + u8 flag = fromwire_u8(cursor, max); + + switch(flag) { + case 0xff: + return fromwire_u64(cursor, max); + case 0xfe: + return (u64)fromwire_u32(cursor, max); + case 0xfd: + return (u64)fromwire_u16(cursor, max); + default: + return (u64)flag; + } +} + void fromwire_pubkey(const u8 **cursor, size_t *max, struct pubkey *pubkey) { u8 der[PUBKEY_DER_LEN]; diff --git a/wire/towire.c b/wire/towire.c index 35bcb6bcdef2..2f36d5f5cf86 100644 --- a/wire/towire.c +++ b/wire/towire.c @@ -54,6 +54,22 @@ void towire_bool(u8 **pptr, bool v) towire(pptr, &val, sizeof(val)); } +void towire_var_int(u8 **pptr, const u64 val) +{ + if (val < 0xfd) { + towire_u8(pptr, (u8)val); + } else if (val <= 0xffff) { + towire_u8(pptr, 0xfd); + towire_u16(pptr, (u16)val); + } else if (val <= 0xffffffff) { + towire_u8(pptr, 0xfe); + towire_u32(pptr, (u32)val); + } else { + towire_u8(pptr, 0xff); + towire_u64(pptr, val); + } +} + void towire_pubkey(u8 **pptr, const struct pubkey *pubkey) { u8 output[PUBKEY_DER_LEN]; diff --git a/wire/wire.h b/wire/wire.h index c72c350a2ba0..69a8a209d5c9 100644 --- a/wire/wire.h +++ b/wire/wire.h @@ -67,6 +67,7 @@ void towire_u64(u8 **pptr, u64 v); void towire_double(u8 **pptr, const double *v); void towire_pad(u8 **pptr, size_t num); void towire_bool(u8 **pptr, bool v); +void towire_var_int(u8 **pptr, const u64 val); void towire_u8_array(u8 **pptr, const u8 *arr, size_t num); @@ -83,6 +84,7 @@ u32 fromwire_u32(const u8 **cursor, size_t *max); u64 fromwire_u64(const u8 **cursor, size_t *max); void fromwire_double(const u8 **cursor, size_t *max, double *v); bool fromwire_bool(const u8 **cursor, size_t *max); +u64 fromwire_var_int(const u8 **cursor, size_t *max); void fromwire_secret(const u8 **cursor, size_t *max, struct secret *secret); void fromwire_privkey(const u8 **cursor, size_t *max, struct privkey *privkey); void fromwire_pubkey(const u8 **cursor, size_t *max, struct pubkey *pubkey); From b58df84f5a92ecdec976dc2d91d20ecac56a0906 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Tue, 2 Apr 2019 19:30:48 -0700 Subject: [PATCH 21/21] tlv: use `var_int`s for size of messages TLV's use var_int's for messages sizes, both internally and in the top level (you should really stack a var_int inside a var_int!!) this updates our automagick generator code to understand 'var_ints' --- tools/generate-wire.py | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 4e2ddbe1a571..040b22082268 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -28,7 +28,8 @@ 'u32': 4, 'u16': 2, 'u8': 1, - 'bool': 1 + 'bool': 1, + 'var_int': 8, } # These struct array helpers require a context to allocate from. @@ -46,8 +47,11 @@ class FieldType(object): def __init__(self, name): self.name = name + def is_var_int(self): + return self.name == 'var_int' + def is_assignable(self): - return self.name in ['u8', 'u16', 'u32', 'u64', 'bool', 'struct amount_msat', 'struct amount_sat'] or self.name.startswith('enum ') + return self.name in ['u8', 'u16', 'u32', 'u64', 'bool', 'struct amount_msat', 'struct amount_sat', 'var_int'] or self.name.startswith('enum ') # We only accelerate the u8 case: it's common and trivial. def has_array_helper(self): @@ -59,6 +63,8 @@ def base(self): basetype = basetype[7:] elif basetype.startswith('enum '): basetype = basetype[5:] + elif self.name == 'var_int': + return 'u64' return basetype # Returns base size @@ -160,8 +166,12 @@ def __init__(self, message, name, size, comments, prevname, includes): # Bolts use just a number: Guess type based on size. if options.bolt: - base_size = int(size) - self.fieldtype = Field._guess_type(message, self.name, base_size) + if size == 'var_int': + base_size = 8 + self.fieldtype = FieldType(size) + else: + base_size = int(size) + self.fieldtype = Field._guess_type(message, self.name, base_size) # There are some arrays which we have to guess, based on sizes. tsize = FieldType._typesize(self.fieldtype.name) if base_size % tsize != 0: @@ -352,8 +362,8 @@ def checkLenField(self, field): return for f in self.fields: if f.name == field.lenvar: - if f.fieldtype.name != 'u16' and options.bolt: - raise ValueError('Field {} has non-u16 length variable {} (type {})' + if not (f.fieldtype.name == 'u16' or f.fieldtype.name == 'var_int') and options.bolt: + raise ValueError('Field {} has non-u16 and non-var_int length variable {} (type {})' .format(field.name, field.lenvar, f.fieldtype.name)) if f.is_array() or f.needs_ptr_to_ptr(): @@ -399,7 +409,7 @@ def print_tlv_fromwire(self, tlv_name): to populate, instead of fields, as well as a length to read in """ ctx_arg = 'const tal_t *ctx, ' if self.has_variable_fields else '' - args = 'const u8 **cursor, size_t *plen, const u16 len, struct tlv_msg_{name} *{name}'.format(name=self.name) + args = 'const u8 **cursor, size_t *plen, const u64 len, struct tlv_msg_{name} *{name}'.format(name=self.name) fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] subcalls = CCode() for f in self.fields: @@ -482,11 +492,13 @@ def print_fromwire(self, is_header, tlv_name): args.append(', {} {}{}'.format(f.fieldtype.name, ptrs, f.name)) template = fromwire_header_templ if is_header else fromwire_impl_templ - fields = ['\t{} {};\n'.format(f.fieldtype.name, f.name) for f in self.fields if f.is_len_var] + fields = ['\t{} {};\n'.format(f.fieldtype.base(), f.name) for f in self.fields if f.is_len_var] subcalls = CCode() for f in self.fields: basetype = f.fieldtype.base() + if f.fieldtype.is_var_int(): + basetype = 'var_int' for c in f.comments: subcalls.append('/*{} */'.format(c)) @@ -636,7 +648,7 @@ def print_towire(self, is_header, tlv_name): if f.is_len_var: if f.lenvar_for.is_tlv: # used below... - field_decls.append('\t{0} {1};'.format(f.fieldtype.name, f.name)) + field_decls.append('\t{0} {1};'.format(f.fieldtype.base(), f.name)) else: field_decls.append('\t{0} {1} = tal_count({2});'.format( f.fieldtype.name, f.name, f.lenvar_for.name @@ -833,20 +845,21 @@ def print_struct(self): \t\ttowire_u8(p, {enum}); \t\ttowire_{tlv_name}_{name}(&tlv_msg, {tlv_name}->{name}); \t\tmsg_len = tal_count(tlv_msg); -\t\ttowire_u8(p, msg_len); +\t\ttowire_var_int(p, msg_len); \t\ttowire_u8_array(p, tlv_msg, msg_len); \t\ttal_free(tlv_msg); \t}} """ tlv__type_impl_towire_template = """static void towire__{tlv_name}(const tal_t *ctx, u8 **p, const struct {tlv_name} *{tlv_name}) {{ -\tu8 msg_len; +\tu64 msg_len; \tu8 *tlv_msg; {fields}}} """ -tlv__type_impl_fromwire_template = """static struct {tlv_name} *fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u16 *len) {{ -\tu8 msg_type, msg_len; +tlv__type_impl_fromwire_template = """static struct {tlv_name} *fromwire__{tlv_name}(const tal_t *ctx, const u8 **p, size_t *plen, const u64 *len) {{ +\tu8 msg_type; +\tu64 msg_len; \tsize_t start_len = *plen; \tif (*plen < *len) \t\treturn NULL; @@ -855,7 +868,7 @@ def print_struct(self): \twhile (*plen) {{ \t\tmsg_type = fromwire_u8(p, plen); -\t\tmsg_len = fromwire_u8(p, plen); +\t\tmsg_len = fromwire_var_int(p, plen); \t\tif (*plen < msg_len) {{ \t\t\tfromwire_fail(p, plen); \t\t\tbreak;