This is an automated email from Gerrit.

Antonio Borneo ([email protected]) just uploaded a new patch set to 
Gerrit, which you can find at http://openocd.zylin.com/5659

-- gerrit

commit a16f8b3c87c18db80f62857ab343eb77079cc11d
Author: Antonio Borneo <[email protected]>
Date:   Thu May 14 14:45:27 2020 +0200

    COVER LETTER: rework commands management
    
    Current code uses three layers of information to handle commands:
    
    1) struct command_registration, static info where command are
       described in the C code. Commands registration is customized at
       runtime depending on target, adapter, ...;
    
    2) struct command, created at runtime, is organized as a tree with
       head in the field 'commands' in struct context;
    
    3) real jimtcl commands. Actually only single 'word' commands and
       the first 'word' of a group of multi-word commands are really
       register as jimtcl command; the rest of multi-word commands are
       searched at run-time by walking the tree in 2).
    
    The help is also based on info defined in 1) and collected in 2);
    the help message is searched by walking the tree in 2).
    
    Still in 2) are kept other info regarding the 'command mode' and
    the need to override the current target.
    
    In all this organization there are several inconsistencies in the
    way data are stored or used, with either hacks/optimizations and
    redundancies (see below).
    
    For long time I have tried to change the way the commands get
    registered in 3), but the tight dependencies between 2) and 3)
    prevents getting it with a simple patch.
    I want each command, even a multi-word one, to be an independent
    jim command.
    The purpose is to be able to rename an existing command, or to
    override it with a tcl proc, or to add a tcl proc with the same
    first words of existing openocd commands.
    One example is in the change http://openocd.zylin.com/5548/ where
    a clean and simple solution would require a tcl proc to override
    the command 'adapter speed'.
    
    Here is the proposal:
    remove the tree in layer 2) and directly register the jim commands
    from 1).
    The help has to be redone from scratch to not rely on the tree. I
    rewrote it in tcl keeping exactly the same output we have today.
    The other info in the tree 2) are passed as private data to each
    command, having jim to 'free' them at command delete.
    All this without changing the input data in 1), with a decrease of
    200 lines of code, and solving most of the inconsistencies.
    
    But the patch series is quite invasive and touches parts of code
    that were stable since long time.
    Any review and test to identify either code errors and/or corner
    cases I could have missed is warmly welcome!
    
    I have used some kind of 'internals' of jimtcl. They are all
    exported and visible in 'jim.h', so should be fine using them.
    Such info are 'stable' since 2011 (jimtcl commit 229239b9443c).
    Anyway I have wrapped them in static inline functions to easily
    track and (eventually) update them.
    
    Next steps:
    on top of this series, I'm now reworking the target prefixed
    commands. The idea is to convert the prefix in a real command that
    can prefix any other tcl command. So
     <target1> myproc
    will run the tcl procedure with target overridden, and the useless
     <target1> <target2> <target3> target current
    will report 'target3'.
    Such change will promote the removal of commands duplicated in
    target space, e.g. 'mdw' and '<target1> mdw'. Only command that
    are specific and/or unique for that target should remain as
    prefixed commands.
    
    ==================================================================
    
    The patch series:
    
    The first set of patches fixes some minor bug found during this
    work and removes unused API to decrease the work surface.
    
    Any knowledge about the command tree in 2) is removed from code
    outside src/helper/command.[ch]
    
    Then uniform the command registration using a single helper for
    all the 'types' of commands and perform target override also for
    "native" jim commands (.jim_handler).
    
    Help is moved to tcl. Added help 'delete' functionality to remove
    the help when a command is deleted. Tested extensively against all
    the help and usage text available today in openocd.
    
    Step-by-step obsoleting the command tree inside the file
    src/helper/command.c until can be removed.
    
    Simple example of using a multi-word tcl proc name.
    
    ==================================================================
    
    I have mentioned inconsistencies. Here they are:
    
    - only the first word of a multi-word command is registered as jim
      command. This means, for example, that only "target" is known by
      jim, then a helper dispatch it for "target init", "target types"
      and so on. This prevents tcl from renaming the subcommands or
      adding a tcl proc as subcommand.
    
    - unregistering a root command through any of the functions
      unregister_[all_]commands() doesn't remove the associated jim
      command because we don't check if there are subcommands still
      registered.
    
    - different registration and behaviour per commands 'type':
    a) root level "native" jim commands (.jim_handler), handled
       directly from jim with NULL private data;
    b) root level "simple" commands (.handler), handled through helper
       script_command() with struct command as private data;
    c) not root level commands are all registered with the helper
       command_unknown(), which then calls either .jim_handler() or
       .handler().
    
    - command 'decoding' starts in jim 'interp' but then is completed
      through the tree that is in openocd 'context'. Since it's
      possible to register a command in one context only, it could
      be missing in another context while still visible in jim. Not
      sure this can have real consequences.
    
    - target override get overridden by DAP and CTI! Again no real
      consequences today, but inconsistent!
    
    - target override through context->current_target_override is
      done only for "simple" commands; "native" command read the
      overridden target directly from command->jim_handler_data.
    
    - fake command is added to command tree to add help to tcl procs.
    
    Those hacks are working fine, but understanding the logic is
    though and any extension is error prone.
    
    Change-Id: Ib2744c737b1c6a2d76082b3dbde0dba6c34f2f02
    Signed-off-by: Antonio Borneo <[email protected]>

-- 


_______________________________________________
OpenOCD-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openocd-devel

Reply via email to