Wireshark-bugs: [Wireshark-bugs] [Bug 2909] Support for V.150 Protocol(SSE/SPRT)
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: bugzilla-daemon@xxxxxxxxxxxxx
Date: Mon, 29 Sep 2008 04:33:23 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2909





--- Comment #4 from Jaap Keuter <jaap.keuter@xxxxxxxxx>  2008-09-29 04:33:22 PDT ---
Is this really based on 0.99.6? That is somewhat old, so that will make it
harder for the patches to be applied successfully. If possible create a patch
from svn HEAD or at least 1.1.2 (development release).

These are some problems I've found in a quick review:
All your values_strings MUST be terminated by {0, NULL} tuple. 
Remove all // comment strings, no need to put your name everywhere.
Use of strcmp() is deprecated, please use g_strncmp()
Remove the printf debug code

Why not make packet-v1501.c a regular dissector i.s.o. a plugin?
Why use both names v1501 and sprt ? I recommend using the name sprt, also for
the name of the sourcefile.
Why define sprt_msgid_vals[] then use a large 
+               if(msgid == SPRT_INIT) {
+                       msgid_str = "INIT";
+               } else if(msgid == SPRT_JMINFO) {

We can ignore packet-v1501.c.old in your patch?

Can you supply sample captures to fuzztest this code?


-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.