Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] rev 24074: /trunk/epan/dissectors/ /trun
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Sun, 13 Jan 2008 14:09:48 +0100
Hi,

Comments inline.

Joerg Mayer wrote:

On Sun, Jan 13, 2008 at 11:56:44AM +0000, jake@xxxxxxxxxxxxx wrote:

http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=24074

User: jake
Date: 2008/01/13 11:56 AM

Log:
 From Michael Earnhart:
 Patch to add cdp Power_request and Power_available fields
 This added the support for the 0x0019 and 0x001a power_request and power_available fields.
Submitted patch slightly modified to present summary line and make more robust. 

I've two open points with this patch:
1) Why don't you use proto_tree_add_item?
That's the current substance of the dissector, unfortunately. This patch only adds dissection of certain tags, a further patch should address the overall issue regarding using proto_tree_add_item. 


2) Could you please print the skipped bytes in case 0<power_avail_len<4
   as padding?
As padding? I'm not sure it should be considered padding, more likely a protocol error. Nevertheless, it's a possible addition. 



   Ciao
      Joerg