From: Zdenek S. <zde...@gm...> - 2013-01-14 16:01:11
|
On Mon, Jan 14, 2013 at 4:07 PM, Ales Ledvinka <ale...@re...> wrote: > Hello, > > Skip the Friday replies. Too hastily to be correct. > Now about the option handling inconsistency. > > The -Y and -a with your last 2nd skip now: > 1) -E -P -a result from last (-a) > 2) -a -E -P -a result from last-1 (-P) > 3) -E -a -P result from last (-P) > > 1st asks that's ok. > 2nd asks, does not use input value and does > not use the last occurring option > 3rd asks, does not use the input value and > uses last options. > It took me a while, but I think I've got your point. Please, post "full" commands as examples. I mean, '-a -E -P -a' will result in password being '-a' which is correct and no questions asked. No big deal, but I was almost ready to tell you off. > BTW: What is it that you do not like on the first patch missing key There are two issues with your patch. The first is, as you correctly pointed out, missing kgkey. The second one is user won't be asked about password if, eg. '-P foo' comes in the last, because you're setting 'querypass' variable to zero at places where you shouldn't. Unless, of course, that's on purpose. But later about this one. But there is no way to tell order of arguments. Well, there are two and one is worse than the other. First, you could parse arguments backwards. But doh in the first place, the second wouldn't that affect something? I'm not even sure if it can be done this way, to be honest. Second, you could create "markers" for these arguments, resp. 'marker_a = current_argc' etc. and then determine precedence and weight, but doh?! And don't forget you must ask password/key no matter what. You know, backwards compatibility is a two way street. A bit ironic, isn't it? It seems to me to fix this properly, so called "backwards compatibility" has to be broken. Z. > > > ----- Original Message ----- > From: "Zdenek Styblik" <zde...@gm...> > To: "Ales Ledvinka" <ale...@re...> > Cc: "ipmitool-devel" <ipm...@li...> > Sent: Saturday, January 12, 2013 6:31:43 AM > Subject: Re: Code Review - ID: 3595612 - ask for password once only if used > > On Fri, Jan 11, 2013 at 5:32 PM, Zdenek Styblik > <zde...@gm...> wrote: > [...] >> Anyway, I'll try to think of some elegant solution over the weekend. >> > > Another solution is attached. > >> Z. >> >>> ----- Original Message ----- >>> From: "Zdenek Styblik" <zde...@gm...> >>> To: "ipmitool-devel" <ipm...@li...> >>> Cc: "Ales Ledvinka" <ale...@re...> >>> Sent: Friday, January 11, 2013 1:32:37 PM >>> Subject: Code Review - ID: 3595612 - ask for password once only if used >>> >>> Hello, >>> >>> attached is a patch to prompt for password only once. As for now, >>> ipmitool prompts for password as many times as, eg. '-a', option has >>> been passed. >>> >>> Ales, please note, attached patch is different from the one you've >>> proposed. Is this workable? Another option would be to put the whole >>> getpass()-thing into if() statement. I gave it a bit of thought and >>> I'm not fond of proposed(original) patch. I don't mean it's bad. But >>> these two variants seem much simpler and cleaner(?). >>> >>> Z. |