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

Reply via email to