Li, Aubrey wrote: > Rafael Vanoni wrote: > >> Hi Aubrey >> >> The fix for 2948, looks good, please push it. >> > > Done. > > >> I've posted a webrev @ >> http://cr.opensolaris.org/~rafaelv/ptop-CR/ that >> fixes a number of issues Kuriakose found during code review >> for the wad >> of changes I'm pushing into ON. Please have a look at it when you >> have the time. >> >> Thanks, >> Rafael > > The changes are all good. Many thanks to Kuriakose for the code review. > One minor issue is one variable is added into powertop.c. And I can't find > any where to use it. > ========================================================= > --- old/usr/src/cmd/powertop/common/powertop.c Wed Apr 1 19:17:55 2009 > +++ new/usr/src/cmd/powertop/common/powertop.c Wed Apr 1 19:17:54 2009 > ------------snip------------- > +char *optarg; > =========================================================
optarg is used by strtod(3) when parsing the command line arguments. > Aside from this, we are always annoied by the following warning when run "hg > nit" > =============================================== > Copyright check: > usr/src/cmd/powertop/common/events.c: no copyright message found > usr/src/cmd/powertop/common/suggestions.c: no copyright message found > usr/src/cmd/powertop/common/powertop.h: no copyright message found > usr/src/cmd/powertop/common/battery.c: no copyright message found > usr/src/cmd/powertop/i386/pt_i386.c: no copyright message found > usr/src/cmd/powertop/common/powertop.c: no copyright message found > usr/src/cmd/powertop/sparcv9/pt_sparcv9.c: no copyright message found > usr/src/cmd/powertop/common/cpufreq.c: no copyright message found > usr/src/cmd/powertop/common/util.c: no copyright message found > usr/src/cmd/powertop/amd64/pt_amd64.c: no copyright message found > usr/src/cmd/powertop/common/display.c: no copyright message found > =============================================== > > Can we change > > "Copyright 2009, Sun Microsystems, Inc" > > to > > " Copyright 2009 Sun Microsystems, Inc. All rights reserved. > Use is subject to license terms." > > ? I don't know.. but cddlchk also fails. Rafael
