| Message ID | 20180727183050.13047-1-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [ndctl] ndctl: deprecate undocumented short-options | expand |
> -----Original Message----- > From: Vishal Verma [mailto:vishal.l.verma@intel.com] > Sent: Saturday, July 28, 2018 3:31 AM > To: linux-nvdimm@lists.01.org > Cc: Vishal Verma <vishal.l.verma@intel.com>; Qi, Fuli/斉 福利 > <qi.fuli@jp.fujitsu.com> > Subject: [ndctl PATCH] ndctl: deprecate undocumented short-options > > The inject-smart and monitor man pages refrained from displaying short options for > various arguments (alarms, daemon) due to a lack of a coherent letter that could > be made to relate to the event name. It was expected that the user will always use > the long option for these. Since the OPT_STRING helper refused to take an empty string > for the short option, we used bogus characters for each of them. > > However there is a better way to provide no short options, and that is by using '\0' > for the short option field to OPT_STRING. Replace the bogus characters with '\0' > so that the 'short help' also becomes consistent with the man pages. > > Cc: Cc: QI Fuli <qi.fuli@jp.fujitsu.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > ndctl/inject-smart.c | 7 ++++--- > ndctl/monitor.c | 2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/ndctl/inject-smart.c b/ndctl/inject-smart.c index 006ea2a..edb87e1 > 100644 > --- a/ndctl/inject-smart.c > +++ b/ndctl/inject-smart.c > @@ -73,7 +73,7 @@ OPT_STRING('M', "media-temperature-threshold", \ > ¶m.media_temperature_threshold, \ > "set smart media temperature threshold", \ > "set threshold value for smart media temperature"), \ -OPT_STRING('x', > "media-temperature-alarm", ¶m.media_temperature_alarm, \ > +OPT_STRING('\0', "media-temperature-alarm", > +¶m.media_temperature_alarm, \ > "smart media temperature alarm", \ > "enable or disable the smart media temperature alarm"), \ OPT_STRING('c', > "ctrl-temperature", ¶m.ctrl_temperature, \ @@ -83,7 +83,7 @@ OPT_STRING('C', > "ctrl-temperature-threshold", \ > ¶m.ctrl_temperature_threshold, \ > "set smart controller temperature threshold", \ > "set threshold value for smart controller temperature"), \ -OPT_STRING('y', > "ctrl-temperature-alarm", ¶m.ctrl_temperature_alarm, \ > +OPT_STRING('\0', "ctrl-temperature-alarm", > +¶m.ctrl_temperature_alarm, \ > "smart controller temperature alarm", \ > "enable or disable the smart controller temperature alarm"), > \ OPT_STRING('s', "spares", ¶m.spares, \ @@ -92,13 +92,14 @@ OPT_STRING('s', > "spares", ¶m.spares, \ OPT_STRING('S', "spares-threshold", > ¶m.spares_threshold, \ > "set smart spares threshold", \ > "set a threshold value for smart spares"), \ -OPT_STRING('z', "spares-alarm", > ¶m.spares_alarm, \ > +OPT_STRING('\0', "spares-alarm", ¶m.spares_alarm, \ > "smart spares alarm", \ > "enable or disable the smart spares alarm"), \ OPT_BOOLEAN('f', "fatal", > ¶m.fatal, "inject fatal smart health status"), \ OPT_BOOLEAN('U', > "unsafe-shutdown", ¶m.unsafe_shutdown, \ > "inject smart unsafe shutdown status") > > + > static const struct option smart_opts[] = { > SMART_OPTIONS(), > OPT_END(), > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index b97d1ea..c6419ad 100644 > --- a/ndctl/monitor.c > +++ b/ndctl/monitor.c > @@ -583,7 +583,7 @@ int cmd_monitor(int argc, const char **argv, void *ctx) > "where to output the monitor's notification"), > OPT_FILENAME('c', "config-file", &monitor.config_file, > "config-file", "override the default config"), > - OPT_BOOLEAN('x', "daemon", &monitor.daemon, > + OPT_BOOLEAN('\0', "daemon", &monitor.daemon, > "run ndctl monitor as a daemon"), > OPT_BOOLEAN('u', "human", &monitor.human, > "use human friendly output formats"), Looks good to me. Please feel free to add: Reviewed-by: QI Fuli <qi.fuli@jp.fujitsu.com> Thanks, QI > -- > 2.14.4 > >
diff --git a/ndctl/inject-smart.c b/ndctl/inject-smart.c index 006ea2a..edb87e1 100644 --- a/ndctl/inject-smart.c +++ b/ndctl/inject-smart.c @@ -73,7 +73,7 @@ OPT_STRING('M', "media-temperature-threshold", \ ¶m.media_temperature_threshold, \ "set smart media temperature threshold", \ "set threshold value for smart media temperature"), \ -OPT_STRING('x', "media-temperature-alarm", ¶m.media_temperature_alarm, \ +OPT_STRING('\0', "media-temperature-alarm", ¶m.media_temperature_alarm, \ "smart media temperature alarm", \ "enable or disable the smart media temperature alarm"), \ OPT_STRING('c', "ctrl-temperature", ¶m.ctrl_temperature, \ @@ -83,7 +83,7 @@ OPT_STRING('C', "ctrl-temperature-threshold", \ ¶m.ctrl_temperature_threshold, \ "set smart controller temperature threshold", \ "set threshold value for smart controller temperature"), \ -OPT_STRING('y', "ctrl-temperature-alarm", ¶m.ctrl_temperature_alarm, \ +OPT_STRING('\0', "ctrl-temperature-alarm", ¶m.ctrl_temperature_alarm, \ "smart controller temperature alarm", \ "enable or disable the smart controller temperature alarm"), \ OPT_STRING('s', "spares", ¶m.spares, \ @@ -92,13 +92,14 @@ OPT_STRING('s', "spares", ¶m.spares, \ OPT_STRING('S', "spares-threshold", ¶m.spares_threshold, \ "set smart spares threshold", \ "set a threshold value for smart spares"), \ -OPT_STRING('z', "spares-alarm", ¶m.spares_alarm, \ +OPT_STRING('\0', "spares-alarm", ¶m.spares_alarm, \ "smart spares alarm", \ "enable or disable the smart spares alarm"), \ OPT_BOOLEAN('f', "fatal", ¶m.fatal, "inject fatal smart health status"), \ OPT_BOOLEAN('U', "unsafe-shutdown", ¶m.unsafe_shutdown, \ "inject smart unsafe shutdown status") + static const struct option smart_opts[] = { SMART_OPTIONS(), OPT_END(), diff --git a/ndctl/monitor.c b/ndctl/monitor.c index b97d1ea..c6419ad 100644 --- a/ndctl/monitor.c +++ b/ndctl/monitor.c @@ -583,7 +583,7 @@ int cmd_monitor(int argc, const char **argv, void *ctx) "where to output the monitor's notification"), OPT_FILENAME('c', "config-file", &monitor.config_file, "config-file", "override the default config"), - OPT_BOOLEAN('x', "daemon", &monitor.daemon, + OPT_BOOLEAN('\0', "daemon", &monitor.daemon, "run ndctl monitor as a daemon"), OPT_BOOLEAN('u', "human", &monitor.human, "use human friendly output formats"),
The inject-smart and monitor man pages refrained from displaying short options for various arguments (alarms, daemon) due to a lack of a coherent letter that could be made to relate to the event name. It was expected that the user will always use the long option for these. Since the OPT_STRING helper refused to take an empty string for the short option, we used bogus characters for each of them. However there is a better way to provide no short options, and that is by using '\0' for the short option field to OPT_STRING. Replace the bogus characters with '\0' so that the 'short help' also becomes consistent with the man pages. Cc: Cc: QI Fuli <qi.fuli@jp.fujitsu.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- ndctl/inject-smart.c | 7 ++++--- ndctl/monitor.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-)