> >> As far as I can see we do not need to allocate a list or add this new > >> OPT_STRING_LIST argument type. Just teach the util_<object>_filter() > routines > >> that the 'ident' argument may be a space delimited list. See the attached > patch: > > > > Thank you for your comment. > > > > The OPT_STRING_LIST is copied from git. > > Consider multiple arguments per option should be supported not only in > > monitor and list but also in other commands, as Vishal mentioned: > > "ndctl disable-namespace namespace1.0 namespace2.0 ..." > > In this case there's no "-n" option so the list parsing is already > handled by standard shell argument parsing, i.e. the argv[] array > already has the list split. > > > If you think this feature is not needed in other commands, I will delete > > OPT_STRING_LIST and make a v2 patch. > > I can see OPT_STRING_LIST potentially being useful in other scenarios, > but for the util_<object>_filter functions it seems an awkward fit to > me. We end up doing the parsing twice. Once to parse the space > separated list and again to parse each entry against the object to be > filtered. In this case I think it is cleaner to handle it internally > to the utility functions. It also makes the util functions more > generically useful as I can now use them in scenarios where the string > list is not coming from the command line. > > Ross noted that the attachment got swallowed by the list, so here it > is pasted, please forgive any whitespace damage:
Thank you very much, I made a v2 patch by referring to your sample patch. Going back to the OPT_STRING_LIST, I think it is necessary for ndctl. Because the other options in monitor like --dimm-event, --bus-event also need to support multiple space-separated arguments, these options should be made as a string_list. One more question is that do you think it is necessary for ndctl to copy OPT_FILENAME in parse_option from git? The difference between OPT_FILENAME and OPT_STRING is that there is an additional fix_filename() in OPTION_FILENAME to process file's path. Considering that ndctl monitor also has file options like --logfile, --conf-file, I think we'd better to copy it. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
