Sebastien Roy wrote:
>
>> Sorry, I hadn't generated the webrev. Here are the diffs against my 
>> previous updates.
>>
>> http://cr.opensolaris.org/~sagun/webrev-seb-comments/
>>
>> The webrev against the libpcap source are here:
>>
>> http://cr.opensolaris.org/~sagun/libpcap-review2/
>
> Great.  Only a couple of nits:
>
> pcap-int.h:
>
> * 71-98: These only apply when compiling with DLPI support.  Perhaps 
> Guy can answer this:  Should there be some sort of #ifdef to make sure 
> that these symbols don't pollute the namespace when compiled on other 
> platforms?
>
> * 376-385: Same comment here.
>
When compiling with libdlpi, a #ifdef HAVE_LIBDLPI can be done but for 
just dlpi, a HAVE_DLPI would need to be defined.
>
>
> pcap-libdlpi.c:
>
> * 100: s/not fail, if the/not fail if the/
>
> * 324: dlpi_hd isn't a boolean.
>
> * 326: buffer isn't a boolean, and you don't need to check this since 
> free() can handle a NULL buffer (this is left over from my previous 
> comment).
>
Sorry I confused myself here. I've corrected the comments above. The 
webrev has been updated.

Thanks,

-sagun


>
> Otherwise, looks good.
>
> -Seb


Reply via email to