Looks okay to me Paul
sagun shakya wrote: > Hi, > > I've address all the comments received so far. The webrev for changes > made after the code review comments are at: > > http://cr.opensolaris.org/~sagun/libpcap-postreview/ > > Webrev against sfw gate: > http://cr.opensolaris.org/~sagun/libpcap-sfw/ > > -Thanks, > > Sagun > > Paul Cunningham wrote: > >> Hi Sagun, >> >> See additional comments inline below ... >> >> Otherwise it looks okay to me >> >> Paul >> >> sagun shakya wrote: >> >>> >>> I've address your comments and generated a new webrev. Please see my >>> responses to your comments inline. >>> >>> Webrev for Paul's comments: >>> http://cr.opensolaris.org/~sagun/libpcap-postreview/ >>> >>> Webrev against sfw gate: >>> http://cr.opensolaris.org/~sagun/libpcap-sfw/ >>> >>>> You have copied various .c and .h files into the src, >>>> could these have been patches to the original (in the >>>> tarball) instead? >>>> >>> >>> >>> Do you mean a patch to the libpcap0.9.8 tarball upstream? If so, no. >>> Elaborating a bit more - all the changes patch/new files have been >>> integrated upstream in the main libpcap gate and the libpcap1.0 tree. >>> Since we do not have a confirmed date on when libpcap1.0 is coming >>> out I'm patching libpcap0.9.8. Does that answer your question? >> >> >> What I was try to say was: >> >> You have copied the following files ... >> 81 cp libdlpi.patches/dlpisubs.c $(VER)/. >> 82 cp libdlpi.patches/dlpisubs.h $(VER)/. >> 83 cp libdlpi.patches/pcap-libdlpi.c $(VER)/. >> so if these are in the tarball why have you copied them rather than >> patched them. But from your comment above it sounds as though they are >> 'new' files - so that's okay then. It may be clearer to add a comment >> to say that though. >> >> >> Also in Makefile.sfw, you have run 'autoconf', is there a reason for >> running this - it's not normal (I think) to have to do this - if you >> need to maybe you need to add comment to say why. >> >> And just as an after thought, you might want to consider using >> compiler flag ... >> > Roland Mainz wrote: >> > >> > the "-xstrconst" puts all string literals into >> > read-only memory (e.g. it's shared between processes and >> > won't waste memory then). >> > > -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products General Dynamics Itronix Europe Ltd. Pioneer House Chivers Way Histon, Cambridgeshire, UK, CB24 9NL Ph: +44 (0)1223 200648 FAX: +44 870 4324162 Email: paul.cunningham at tadpole.com This email message is for the sole use of the intended recipient(s) and may contain GDC4S confidential or privileged information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message
