Hi Jack, installadm.c 309 - Since MSG_BAD_SERVICE_NAME contains a \n, won't you be printing two \n's? 306,318 - Why are there curly brackets around the case statement? It looks kind of odd and doesn't match the rest of the case statement. And the closing bracket is in the wrong place anyway, because it encompasses the comment from the next case. 680,756,874, etc. - same comment as 309 1033 - Maybe validate the service name at 1061, instead? This would maintain consistency with how you did it in do_add and do_remove. 1310 - I'd remove the XXX, I don't think it adds anything to the comment.
Thanks, Sue On 03/11/09 14:00, Jack Schwartz wrote: > Hi everyone. > > Fix has been renovated with the following: > - Only alphanumeric chars, underscore and hyphen allowed in service > names, per our bugcourt meeting yesterday. > - I fixed that the service name wasn't checked everywhere necessary. > - Shortened several lines > 80 chars and other stuff like that to get > code hg nits clean. > - Other cleanup. > > Same location: > http://cr.opensolaris.org/~schwartz/090306.1/webrev/ > > Please review. > > Thanks, > Jack > > On 03/06/09 13:41, Jack Schwartz wrote: >> Hi everyone. >> >> Here is a code review for a couple of small bugfixes: >> >> 5091 AI install does not work if your service name had . in it. >> 4610 most installadm commands need to err out gracefully if not root >> >> http://cr.opensolaris.org/~schwartz/090306.1/webrev/ >> >> Please review. >> >> Thanks, >> Jack >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
