Hi, Paul, Thank you for your comments, please see in line.
Paul Cunningham wrote: > Jason, > > See below for my comments .... > > Paul > > Jason Zhao wrote: >> >> Can someone do a review for the webrev of RFE (6811043 Need to port >> snort into Solaris), please. >> >> Bug: >> http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6811043 >> >> Webrev: >> http://cr.opensolaris.org/~jxzhao/snort > > ==== Start of Comments === > > 1. usr/src/cmd/Makefile > & usr/src/pkgdefs/Makefile > You need to resync this with the sfwnv gate otherwise > it looks as though you are try to delete other stuff. Sorry for that, I will sync with sfwnv gate for the Makefile. > > 2. usr/src/cmd/snort/METADATA > Make the NAME: field more descriptive > > Change ... > 7 SRC: > to be the url of the src tarball, ...../snort-2.8.3.1.tar.gz Agree, I will do that. > > 3. usr/src/cmd/snort/Makefile.sfw > Use the predefined configure prefix value CFGPREFIX from > the Makefile.master - your definition is wrong anyway as > '--prefix=$(ROOT)/usr' says at user runtime it will be > running from the workspace's proto area rather than /usr. > So change as follows ... > Delete lines ... > 45 CONFIGURE_OPTIONS += --prefix=$(ROOT)/usr > 41 CONFIGURE_OPTIONS = --mandir=/tmp > then you will get the '--prefix=' and '--mandir=' > from Makefile.master OK, thank you. > > Your lines ... > 49 CONFIGURE_OPTIONS64 += --bindir=$(ROOT)/usr/bin/$(MACH64) > 50 CONFIGURE_OPTIONS64 += --exec_prefix=$(ROOT)/usr/lib/$(MACH64) > I think should also be ... > 49 CONFIGURE_OPTIONS64 += --bindir=$(CFGUSRSBIN64) > 50 CONFIGURE_OPTIONS64 += --exec_prefix=/usr/lib/$(MACH64) Thanks. > > You are building this with 'gcc' will it build with the Sun > compiler (may need patches) ? If it will use the Sun compiler. CC would not work, I got following error while compiling with Sun compiler. ---------------------------------------------------------------- <snip> cc -DHAVE_CONFIG_H -I. -I../../.. -I../../.. -I../../../src -I../../../src/sfutil -I../../../src/output-plugins -I../../../src/detection-plugins -I../../../src/dynamic-plugins -I../../../src/preprocessors -I../../../src/preprocessors/flow -I../../../src/preprocessors/portscan -I../../../src/preprocessors/flow/int-snort -I../../../src/preprocessors/HttpInspect/include -I../../../src/preprocessors/Stream5 -I../../../src/target-based -DBSD_COMP -D_REENTRANT -I/usr/include/pcre -g -DDYNAMIC_PLUGIN -DDETECTION_OPTION_TREE -c snort_stream5_tcp.c "../../../src/rules.h", line 53: warning: macro redefined: INADDR_NONE "snort_stream5_tcp.c", line 2528: reference to static identifier "Stream5SeglistDeleteNode" in extern inline function "snort_stream5_tcp.c", line 3068: reference to static identifier "FlushStream" in extern inline function "snort_stream5_tcp.c", line 3080: reference to static identifier "DeleteSeglist" in extern inline function cc: acomp failed for snort_stream5_tcp.c *** Error code 2 make: Fatal error: Command failed for target `snort_stream5_tcp.o' Current working directory /export/home/Jason/work/pkgport/snort-2.8.3.1/src/preprocessors/Stream5 *** Error code 1 </snip> ---------------------------------------------------------------- I send to development mail list about the issue, I think the maintainer of snort has accepted and will put it on their schedule. > > In install32: & install64:, after running 'make install' do > you need to run 'protofix' to correct any file > permissions/owner/group settings? (see (6)). Not, yet. I will do that, thank you for point it out. > > 4. usr/src/cmd/snort/sunman-stability > Is your > 22 Interface Stability Volatile > correct? I will change it to "Uncommitted" > > 5. usr/src/pkgdefs/SUNWsnort/pkginfo.tmpl > Add the pkg version number to the end of the DESC= line, eg. > DESC=".................. (2.8.3.1)" Thank you. > > 6. usr/src/pkgdefs/SUNWsnort/prototype_com > & usr/src/pkgdefs/SUNWsnort/prototype_i386 > & usr/src/pkgdefs/SUNWsnort/prototype_sparc > Do not install files in /usr with the write permission > bit set. I will use protofix to check them. Thank you Jason
