On 25.11.2014 14:39, Bruce Richardson wrote: > On Tue, Nov 25, 2014 at 01:45:22PM +0100, Thomas Monjalon wrote: >> Hi Simon, >> >> 2014-11-25 10:09, Simon Kuenzer: >>> thanks for your work. I have one (minor) comment for this patch that >>> should be fixed in a later version. >> >>>> + /* default master lcore is the first one */ >>>> + if (cfg->master_lcore == 0) >>>> + cfg->master_lcore = rte_get_next_lcore(-1, 0, 0); >>>> + >>> >>> Might be confusing if a user specifies --master-lcore 0 and uses a >>> coremask/corelist where core id 0 is not specified. >> >> Yes, in this corner case, master-lcore will be adjusted instead of having >> an error. >> >>> What about setting cfg->master_lcore to (RTE_MAX_LCORE + 1) on >>> initialization in order to distinguish if a master_lcore got specified >>> by the user or not? >> >> Even simpler, I can fix it by introducing a flag master_lcore_parsed and >> do the adjustment only if the option is not parsed. >> > I agree that that sounds like a simpler approach, since we already have flags > for what args are parsed or not. > > /Bruce >
Fine with me :-). I also agree that having the flag is even a cleaner solution. Thanks, Simon
