Thank you for reviewing the changes Guy!

FYI, I've also filed bug # 1882884 " Add libpcap support for new Solaris 
features".

Guy Harris wrote:
>
>> I mostly fixed the comment changes, thinking they were trivial and 
>> left other Sun cstyle error alone. I can see how it is distracting 
>> while reviewing  the actual changes. I've undone such changes and 
>> posted a new webrev at:
>>
>> http://cr.opensolaris.org/~sagun/libpcap/
>
> pcap-streams.c isn't purely STREAMS-related, and most of the 
> pcap-XXX.c files have implementations of the libpcap APIs; you might 
> want to call it, for example, "dlpisubs.c", as it's subroutines for 
> both the "raw DLPI" and libdlpi-mediated DLPI implementations.
Accepted. I'll change pcap-streams.c pcap-dlpisubs.c.

>
> pcap_read_pkt() might be better named pcap_read_pkts(), or 
> pcap_process_pkts() - it processes more than one packet.
pcap_process_pkts fits more to the function. I've fixed it to be that.
>
> pcap_set_linktype() doesn't set the link-layer type in the sense that 
> pcap_set_datalink() does - i.e., it doesn't change what the link-layer 
> type of the packets supplied by the device is.  pcap_process_mactype() 
> might be a better name.
Accepted.
>
> pcap_conf_bufmod() also flushes the read side of the stream; you might 
> want to do that outside that routine, as that's not related to setting 
> up bufmod.  Then pcap_conf_bufmod() wouldn't exist on platforms 
> without bufmod.
>
Yes, I had split pcap_alloc_databuf() for similar reason. Instead of 
creating another routine I'll just do this exclusively in 
pcap-[dlpi,libdlpi].c
> You might also want to have pcap_open_live() in pcap-dlpi.c set 
> passive mode if available, so that libpcap-based applications can 
> coexist with link aggregation even in pre-Solaris 11 versions.
I'll add this to pcap_open_live in pcap_dlpi.c

I'll send an updated webrev once I've updated these changes.

-thanks,

Sagun

-- 

Sagun Shakya
781.442.7344/ X27344
sagun.shakya at sun.com



Reply via email to