From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3262B43155; Fri, 13 Oct 2023 14:23:19 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D0A81402D3; Fri, 13 Oct 2023 14:23:18 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 82516402CF for ; Fri, 13 Oct 2023 14:23:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1697199797; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VOcvy5uJTM5soUOBVuNsQAvngrSuPIXxYweVI0fJBbw=; b=JOVnVQN2XeFh4+Eeq52kL5uRzioSQdsU4fcwdXAlj6UGjoGIKu2fLM6pnJ7BKmbYEo44cS Ow4MscwnPnlAywxB9CnvkRxiipJCVpJX2uQwWt6uy37oeTjNct2Q9zWuPLBRWbfwFiSUl8 RxtdT8stpyt2mRVKoIu/O11MtOrrGJA= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-376-59Q8jaLHPQ2S0HkKoBggwQ-1; Fri, 13 Oct 2023 08:23:15 -0400 X-MC-Unique: 59Q8jaLHPQ2S0HkKoBggwQ-1 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-32d931872cbso909358f8f.0 for ; Fri, 13 Oct 2023 05:23:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697199794; x=1697804594; h=in-reply-to:references:to:from:subject:message-id:date :content-transfer-encoding:mime-version:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=qZ1FFYxWs4fRD3lExJ+UE/sMxQqPIvJ8RjejGxac7Ao=; b=dFtapEJ6yoe3wxCFB6AvilADy88U3vIhvLj2Q9fZr1big1r+Xl1m0Uz0s9HZfHNL4p iaaStw8ymsCNvUZ2WFLRJniIwkdIWTsZ+Hhv2QPwTpuz1Rud36nHFpUDcgCzQCnCgC4+ gi8CyJt8m5q/+pTFIEUmRc2SHNDGsVVHk8cWrjFzgu7J90ocStfB1WQ4OMq1pPHTuD4Q S4H3Ng/rPq70J6G+wxANudfVuxtyip0n6JeSehpUlvFBtN9UyV6lFZCM+MyiTzPcOXw/ IB/IlrMUy5DQF6/TOlAfu0jgJ8PrMkAWrfMEDrIZm2zqdWwwnGqIAKIU8wVKWK2ozoZ3 qKEw== X-Gm-Message-State: AOJu0YwldGgOGsksWnmozr7h8R9ehpVAtVz9YEJRTysXYygDhMHSUCNL meXs2kpiB9eqp0XCD8pkmTXVgh19m/5O8gj3keU8CTi2XxNv+HZ5k8nT2QDH0xuj5lWbIipmaUT j/NM= X-Received: by 2002:adf:a387:0:b0:32d:8894:6aa2 with SMTP id l7-20020adfa387000000b0032d88946aa2mr6220762wrb.2.1697199794497; Fri, 13 Oct 2023 05:23:14 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFO+DFB7P//LpRhZdERWeEcoQdWGZFlWIZwchpkMubl5mKqqvFFqzXHKMsGBZVp9jVpBGpVwg== X-Received: by 2002:adf:a387:0:b0:32d:8894:6aa2 with SMTP id l7-20020adfa387000000b0032d88946aa2mr6220743wrb.2.1697199794049; Fri, 13 Oct 2023 05:23:14 -0700 (PDT) Received: from localhost (2a01cb000f8b9700598da2e1679e8383.ipv6.abo.wanadoo.fr. [2a01:cb00:f8b:9700:598d:a2e1:679e:8383]) by smtp.gmail.com with ESMTPSA id n6-20020adffe06000000b003140f47224csm20639014wrr.15.2023.10.13.05.23.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 13 Oct 2023 05:23:13 -0700 (PDT) Mime-Version: 1.0 Date: Fri, 13 Oct 2023 14:23:13 +0200 Message-Id: Subject: Re: [PATCH v3 2/5] buildtools: script to generate cmdline boilerplate From: "Robin Jarry" To: "Bruce Richardson" , X-Mailer: aerc/0.16.0-22-gb445f7bb08fb-dirty References: <20230802170052.955323-1-bruce.richardson@intel.com> <20231011133357.111058-1-bruce.richardson@intel.com> <20231011133357.111058-3-bruce.richardson@intel.com> In-Reply-To: <20231011133357.111058-3-bruce.richardson@intel.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8; format=Flowed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Bruce Richardson, Oct 11, 2023 at 15:33: > Provide a "dpdk-cmdline-gen.py" script for application developers to > quickly generate the boilerplate code necessary for using the cmdline > library. > > Example of use: > The script takes an input file with a list of commands the user wants in > the app, where the parameter variables are tagged with the type. > For example: > > =09$ cat commands.list > =09list > =09add x y > =09echo message > =09add socket path > =09quit > > When run through the script as "./dpdk-cmdline-gen.py commands.list", > the output will be the contents of a header file with all the > boilerplate necessary for a commandline instance with those commands. > > If the flag --stubs is passed, an output header filename must also be > passed, in which case both a header file with the definitions and a C > file with function stubs in it is written to disk. The separation is so > that the header file can be rewritten at any future point to add more > commands, while the C file can be kept as-is and extended by the user > with any additional functions needed. > > Signed-off-by: Bruce Richardson > --- Hi Bruce, this is a nice addition, I have a few python style remarks below. In general, I would advise formatting your code with black[1] to avoid=20 debates over coding style. It makes all code homogeneous and lets you=20 focus on more important things :) > buildtools/dpdk-cmdline-gen.py | 167 ++++++++++++++++++++++++++++++ > buildtools/meson.build | 7 ++ > doc/guides/prog_guide/cmdline.rst | 131 ++++++++++++++++++++++- > 3 files changed, 304 insertions(+), 1 deletion(-) > create mode 100755 buildtools/dpdk-cmdline-gen.py > > diff --git a/buildtools/dpdk-cmdline-gen.py b/buildtools/dpdk-cmdline-gen= .py > new file mode 100755 > index 0000000000..3b41fb0493 > --- /dev/null > +++ b/buildtools/dpdk-cmdline-gen.py > @@ -0,0 +1,167 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2023 Intel Corporation > +# > +"""Script to automatically generate boilerplate for using DPDK cmdline l= ibrary.""" Multi line (or single line) doc strings are usually formatted as=20 follows: """ Script to automatically generate boilerplate for using DPDK cmdline library= . """ It makes adding new lines more readable and saves a bit of characters=20 per line. > + > +import argparse > +import sys > + > +PARSE_FN_PARAMS =3D 'void *parsed_result, struct cmdline *cl, void *data= ' > +PARSE_FN_BODY =3D """ > + /* TODO: command action */ > + RTE_SET_USED(parsed_result); > + RTE_SET_USED(cl); > + RTE_SET_USED(data); > +""" > + > + > +def process_command(tokens, cfile, comment): > + """Generate the structures and definitions for a single command.""" > + name =3D [] > + > + if tokens[0].startswith('<'): > + print('Error: each command must start with at least one literal = string', file=3Dsys.stderr) > + sys.exit(1) It would be better to raise an exception here and handle it in main()=20 for error reporting. > + for t in tokens: > + if t.startswith('<'): > + break > + name.append(t) > + name =3D '_'.join(name) > + > + result_struct =3D [] > + initializers =3D [] > + token_list =3D [] > + for t in tokens: > + if t.startswith('<'): > + t_type, t_name =3D t[1:].split('>') > + t_val =3D 'NULL' > + else: > + t_type =3D 'STRING' > + t_name =3D t > + t_val =3D f'"{t}"' > + > + if t_type =3D=3D 'STRING': > + result_struct.append(f'\tcmdline_fixed_string_t {t_name};') > + initializers.append( > + f'static cmdline_parse_token_string_t cmd_{name}_{t_= name}_tok =3D\n' + > + f'\tTOKEN_STRING_INITIALIZER(struct cmd_{name}_resul= t, {t_name}, {t_val});') > + elif t_type in ['UINT8', 'UINT16', 'UINT32', 'UINT64', 'INT8', '= INT16', 'INT32', 'INT64']: > + result_struct.append(f'\t{t_type.lower()}_t {t_name};') > + initializers.append( > + f'static cmdline_parse_token_num_t cmd_{name}_{t_nam= e}_tok =3D\n' + > + f'\tTOKEN_NUM_INITIALIZER(struct cmd_{name}_result, = {t_name}, RTE_{t_type});') > + elif t_type in ['IP', 'IP_ADDR', 'IPADDR']: > + result_struct.append(f'\tcmdline_ipaddr_t {t_name};') > + initializers.append( > + f'cmdline_parse_token_ipaddr_t cmd_{name}_{t_name}_t= ok =3D\n' + > + f'\tTOKEN_IPV4_INITIALIZER(struct cmd_{name}_result,= {t_name});') > + else: > + print(f'Error: unknown token-type {t}', file=3Dsys.stderr) > + sys.exit(1) > + token_list.append(f'cmd_{name}_{t_name}_tok') > + > + print(f'/* Auto-generated handling for command "{" ".join(tokens)}" = */') > + # output function prototype > + func_sig =3D f'void\ncmd_{name}_parsed({PARSE_FN_PARAMS})' > + print(f'extern {func_sig};\n') > + # output function template if C file being written > + if (cfile): > + print(f'{func_sig}\n{{{PARSE_FN_BODY}}}\n', file=3Dcfile) > + # output result data structure > + print( > + f'struct cmd_{name}_result {{\n' + > + '\n'.join(result_struct) + > + '\n};\n') > + # output the initializer tokens > + print('\n'.join(initializers) + '\n') > + # output the instance structure > + print( > + f'static cmdline_parse_inst_t cmd_{name} =3D {{\n' + > + f'\t.f =3D cmd_{name}_parsed,\n' + > + '\t.data =3D NULL,\n' + > + f'\t.help_str =3D "{comment}",\n' + > + '\t.tokens =3D {') > + for t in token_list: > + print(f'\t\t(void *)&{t},') > + print('\t\tNULL\n' + '\t}\n' + '};\n') > + > + # return the instance structure name > + return f'cmd_{name}' > + > + > +def process_commands(infile, hfile, cfile, ctxname): > + """Generate boilerplate output for a list of commands from infile.""= " > + instances =3D [] > + > + # redirect stdout to output the header, to save passing file=3D each= print > + old_sys_stdout =3D sys.stdout > + sys.stdout =3D hfile Why not use hfile.write()? I think the main issue here is to use print() in process_commands(). It=20 would probably be cleaner to have process_command() return a list of=20 lines and print them in this function. > + > + print(f'/* File autogenerated by {sys.argv[0]} */') > + print('#ifndef GENERATED_COMMANDS_H') > + print('#define GENERATED_COMMANDS_H') > + print('#include ') > + print('#include ') > + print('#include ') > + print('#include ') > + print('#include ') > + print('') You can use a multi-line f-string here with a single print/write. hfile.write(f"""/* File autogenerated by {sys.argv[0]} */ #ifndef GENERATED_COMMANDS_H #define GENERATED_COMMANDS_H #include #include #include #include #include """) > + > + for line in infile.readlines(): > + if line.lstrip().startswith('#'): > + continue > + if '#' not in line: > + line =3D line + '#' # ensure split always works, even if no= help text > + tokens, comment =3D line.split('#', 1) > + instances.append(process_command(tokens.strip().split(), cfile, = comment.strip())) If process_command returns a name and a list of lines, that could be=20 transformed as: name, lines =3D process_command(tokens.strip().split(), cfile, c= omment.strip()) instances.append(name) hfile.write("\n".join(lines) + "\n") > + > + print(f'static __rte_used cmdline_parse_ctx_t {ctxname}[] =3D {{') > + for inst in instances: > + print(f'\t&{inst},') > + print('\tNULL') > + print('};\n') > + print('#endif /* GENERATED_COMMANDS_H */') also multi line print here: hfile.write("""\tNULL }; #endif /* GENERATED_COMMANDS_H */ """) > + > + sys.stdout =3D old_sys_stdout > + > + > +def main(): > + """Application main entry point.""" > + ap =3D argparse.ArgumentParser() Nit to have a nice description of the command with --help: ap =3D argparse.ArgumentParser(description=3D__doc__) > + ap.add_argument( > + '--stubs', action=3D'store_true', > + help=3D'Produce C file with empty function stubs for each co= mmand') > + ap.add_argument( > + '--output-file', '-o', default=3D'-', > + help=3D'Output header filename [default to stdout]') > + ap.add_argument( > + '--context-name', default=3D'ctx', > + help=3D'Name given to the cmdline context variable in the ou= tput header [default=3Dctx]') > + ap.add_argument( > + 'infile', type=3Dargparse.FileType('r'), > + help=3D'File with list of commands') > + args =3D ap.parse_args() > + > + if not args.stubs: > + if args.output_file =3D=3D '-': > + process_commands(args.infile, sys.stdout, None, args.context= _name) > + else: > + with open(args.output_file, 'w') as hfile: > + process_commands(args.infile, hfile, None, args.context_= name) > + else: > + if not args.output_file.endswith('.h'): > + print( > + 'Error: output filename must end with ".h" extension= when creating stubs', > + file=3Dsys.stderr) > + sys.exit(1) You can replace print to stderr + exit with: ap.error("-o/--output-file: must end with .h extension when creating stubs"= ) > + > + cfilename =3D args.output_file[:-2] + '.c' > + with open(args.output_file, 'w') as hfile: > + with open(cfilename, 'w') as cfile: > + print(f'#include "{args.output_file}"\n', file=3Dcfile) > + process_commands(args.infile, hfile, cfile, args.context= _name) > + > + > +if __name__ =3D=3D '__main__': > + main() I'll stop here ;) Thanks!