On Tue, Apr 2, 2019 at 5:30 PM Varghese, Vipin <vipin.vargh...@intel.com> wrote:
> Hi David, > > > > snipped > #define CMD_LINE_OPT_PDUMP "pdump" > +#define CMD_LINE_OPT_PDUMP_NUM 1 > +#define CMD_LINE_OPT_MULTI "multi" > +#define CMD_LINE_OPT_MULTI_NUM 2 > #define PDUMP_PORT_ARG "port" > #define PDUMP_PCI_ARG "device_id" > #define PDUMP_QUEUE_ARG "queue" > > > > You'd better map to integers that do not collide with ascii characters > (even if non printable). > > So values >= 256 are better. > > This is the reason for the comment here: > > https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/eal_options.h#n19 > > > > In case of dpdk-pdump; there are 2 options ‘multi’ and ‘pdump’. But in > case of eal_option there are multiple options which has same fist > character. Hence the comment 'first long only option value must be >= 256, > so that we won't conflict with short options' is true. > > > > In my humble opinion, I think it need not change. But please let me know > otherwise. > The convention I had proposed is to just leave the whole [0-255] range to potential short options. This marks a demarcation between long options that map to short options and long "only" options. I don't care, just prefer to have something systematic. Snipped > > + printf("usage: %s [EAL options] -- [--%s] " > + "--%s " > "'(port=<port id> | device_id=<pci id or vdev > name>)," > "(queue=<queue_id>)," > "(rx-dev=<iface or pcap file> |" > @@ -152,7 +157,7 @@ pdump_usage(const char *prgname) > "[ring-size=<ring size>default:16384]," > "[mbuf-size=<mbuf data size>default:2176]," > "[total-num-mbufs=<number of > mbufs>default:65535]'\n", > - prgname); > + prgname, CMD_LINE_OPT_MULTI, CMD_LINE_OPT_PDUMP); > } > > static int > > > > You can concatenate the macro. > > > > Thank you for the suggestion, > > > > #define OPT(X) #X > > #define STR_REPLACE "\n[--"OPT(multi)"]\n --"OPT(pdump) > > > prgname, STR_REPLACE); > > > > should we change to this or skip this as we are using this once? > > > > snipped > I don't understand what you propose. My suggestion is simple: + printf("usage: %s [EAL options] -- [--"CMD_LINE_OPT_MULTI"] " + "--"CMD_LINE_OPT_PDUMP" " -- David Marchand
