On Wed, Apr 18, 2018 at 3:09 AM, Qi, Fuli <qi.f...@jp.fujitsu.com> wrote: >> >> 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.
Ok, yes, lets bring in string_list for that case. > 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. Yes, let's copy git's lead for filename fixups. _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
