Hi Richard, On Wed, 2015-08-12 at 21:45 +0200, Richard Cochran wrote: > Changes in V2: > - API is much simpler to use > - numerous details in the implementation have been improved > - half dozen legacy options converted >
Nice changes. Looks very straight forward to use. > The config code was simplistic at first and has grown many warts over > time. The pointers to global variables are rather gross, and it is > hard to add new options, especially port specific ones. The number > of > configuration options is sure to grow larger over time. > Yep. > This series addresses the issues by implementing a hash table based > implementation. Adding a new option is as simple as placing the item > into the config_tab[] table (one line), and then calling > var=config_get_TYPE() at the usage site. Nice. Super easy to add, and it looks like the various modes support most of our current options. > > This series add one new option and converts six legacy options. Many > if not all of the remaining legacy options can be converted to the > new > system. Once this is done, the config code will be much more compact > and maintainable. In addition, the phc2sys and pmc programs will > also > be able to use a configuration file. > Very useful. Will take some time to setup all the options for each file. > Patches 1-4 and 12 provide changes allowing passing of the > configuration. > Patch 5 and 6 add a simple hash table. > Patch 7 adds the bulk of the new implementation. > Patch 8 lets the UDP transport use the new TTL option. > Patches 9-11 and 14-16 convert some legacy options. > Patch 13 adds a way to "lock" options from the command line. > > Review and comments are most welcome. General comments: it doesn't appear to enforce any specific change on configuration file format. I know my earlier proposal was maybe to at some point update the format. Specifically, change the per-port configurations to use "port:" or some prefix which might help ensure possible future expansion of sections. The primary reasoning being separating out options used by differing programs. It seems that is something we could handle in the future, though. Are there any options you think might be more difficult to convert? I think some of the string options might not move as cleanly to the new API.. each call site would have to parse the string instead of parsing an enumeration. Maybe that can be solved by having a GLOB_ITEM_ENUM or something where the setup passes in a map of string->value that could be used instead, so that callsites get an enum/int of values instead of strings? Other comment is whether we want phc2sys and ptp4l to read from the same configuration file or should they just read separate ones. Aside from my general comments, I looked over the series and it looks great. Most of my comments here wouldn't sway me to say don't apply it now. We can always continue to work towards moving all the options from legacy into the new format. Regards, Jake ------------------------------------------------------------------------------ _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
