Guy Harris wrote: > On Feb 1, 2008, at 12:47 PM, sagun shakya wrote: > > >> Sebastien Roy wrote: >> >>> * 118-123: I would think that in 2008, most compilers can do a >>> better job at assigning register use than this (making every single >>> variable a "register" variable.) You don't have to fix this; this >>> is more a question for those more knowledgeable than me at compiler >>> optimization. >>> >> Since I'm not an expert either, I wasn't sure what to do here and in >> the new file added. I was expecting there would be some comment >> regarding how it would be best to use them. >> > > My comment, as per my other mail, about how it's best to use > "register" is "it's best not to use 'register' these days". Let the > compiler worry about it. > Ok. I'll remove the use of 'register' from my changes.
>>> * 130: This looks like a bug. In the pcap-dlpi.c code, failure to >>> enable multicast promiscuous mode does not result in a failure, but >>> only a warning. >>> >> Will fix, but before that I have a question,I had changed the logic >> here in comparision to pcap-dlpi.c; in pcap-dlpi.c the logic is : >> if (promisc) >> enable physical promiscuous mode >> then enable multicast promiscuous mode >> else >> enable promiscuous mode at SAP level. >> >> In pcap-libdlpi I have it to be, : >> if (promisc) >> enable physical promiscuous mode >> else >> enable multicast promiscuous mode >> >> enable promiscuous mode at SAP level. >> >> I guessthe logic should be as in pcap-dlpi.c? >> > > Perhaps. I don't know what the comment in the code sample from my > previous mail: > > /* > ** Try to enable multicast (you would have thought > ** promiscuous would be sufficient). (Skip if using > ** HP-UX or SINIX) (Not necessary on send FD) > */ > > is referring to. pcap-dlpi.c supports several platforms: > > SunOS 5.x, for various values of "x" dating back at least to 2, given > that there's code to work around a bufmod bug that checks for SunOS > 5.2 as well as 5.3.0 and 5.3.1; > > HP-UX 9, 10, and 11; > > AIX (although, by default, BPF is used); > > SINIX > > and, for all I know, it might support some other SV-ish UN*Xes as > well. I don't know which versions of which OSes had that issue, > wherein you had to turn on DL_PROMISC_MULTI as well as DL_PROMISC_PHYS > (that code predates my involvement with libpcap - it dates back to > libpcap 0.4). Obviously neither HP-UX nor SINIX had it, as per the > comment and the #ifdefs that omit the code in question on those OSes; > I don't know if it's required by AIX, older versions of SunOS 5.x, or > all versions of SunOS 5.x. > Since we know what libldpi supports and it is not needed to turn on DL_PROMISC_MULTI as well as DL_PROMISC_PHYS, I will leave the logic that I have in pcap-libdlpi.c and leave pcap-dlpi.c as it is. Thanks, Sagun
