Roland, Comments inlined. Roland Mainz: > Ling-Jun Simon Wang wrote: > >> <a href=http://cr.opensolaris.org/~lw202042/smartmontools/>code review for >> smartmontools</a> >> > > Quick 5min race through > http://cr.opensolaris.org/~lw202042/smartmontools/sfw.patch (patch code > is quoted with "> "): > --- /dev/null Wed Apr 16 11:53:11 2008 > +++ new/usr/src/cmd/smartmontools/Makefile.sfw Wed Apr 16 11:43:25 2008 > @@ -0,0 +1,64 @@ > [snip] > +all: $(VER)/config.status > + (cd $(VER); env \ > + CC=$(CC) \ > + INSTALL=/usr/ucb/install \ > + "CFLAGS=$(CFLAGS) -xc99=none -xCC" \ > > 1. Please explain why "-xc99" must be set to "none" ? It looks a bit odd > for me... > Because there exist some c files in the project so I added these flags. But a close look at the code reveals no cpp-style comments in these c files. As a result this flag can be removed. > 2. Please add "-xO3" if "configure" does not enable the optimizer > itself... > 3. Please add "-xstrconst" > > These are OK. I add them. >> +$(VER)/config.status: $(VER)/configure >> + (cd $(VER); env \ >> + INSTALL=/usr/ucb/install \ >> + PATH=$(SFW_PATH) \ >> + ./configure --prefix=/) >> > > "CC" and "CFLAGS" must be set for "configure", too (e.g. $ > CC=path_to_my_compiler CFLAGS="myflags" ./configure #), otherwise the > autoconf probes and the CC/CFLAGS used by the "make" run will differ > I found if CC or CFLAGS is set, the project can not be made in the default configuration. This is why I removed them.
Thanks for your review. - Simon > ---- > > Bye, > Roland > >
