http://blog.gmane.org/gmane.network.wireshark.bugs hourly 1 1901-01-01T00:00+00:00 http://gmane.org/img/gmane-25t.png http://gmane.org http://permalink.gmane.org/gmane.network.wireshark.bugs/11143 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2922 Gerald Combs <gerald< at >wireshark.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Group|private | --- Comment #2 from Gerald Combs <gerald< at >wireshark.org> 2008-10-07 13:32:42 PDT --- Making public, with David's permission. bugzilla-daemon< at >wireshark.org 2008-10-07T20:32:42 http://permalink.gmane.org/gmane.network.wireshark.bugs/11142 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2924 Gerald Combs <gerald< at >wireshark.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Group|private | --- Comment #6 from Gerald Combs <gerald< at >wireshark.org> 2008-10-07 13:32:20 PDT --- Making public, with David's permission. bugzilla-daemon< at >wireshark.org 2008-10-07T20:32:21 http://permalink.gmane.org/gmane.network.wireshark.bugs/11141 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2935 --- Comment #9 from Bill Meier <wmeier< at >newsguy.com> 2008-10-07 11:15:20 PDT --- First: Thanks for your effort so far on this dissector ! Some additional comments: 1. I don't believe the 'if (..._tree)' for the various subtrees is req'd since proto_item_add_subtree will always return a non-null value (as long as the parent is non-null) 2. The following don't appear to be used: - mikey_tcp_port - proto_register(): 'int i = 0;' 3. pvaluedec is defined as a guint16 and is later calculated as 100 * <a guint16>. This seems like a potential for overflow. 4. The 'if (...==-1)' in proto_register.... is not needed. 5. The code to build 'param_value' uses strcat and sprintf which are "memory unsafe" functions and thus not allowed in Wireshark. I suggest instead displaying the "param value" fields by using FT_BYTES and BASE_HEX in the relevant hf[] entries. If you would rather have the hex strings shown with spaces between each byte, then I guess you'll need to build param_value as now but instead maybe using g_strlcat & g_snprintf. See the glib reference for info on g_strlcat &etc (http://library.gnome.org/devel/glib/2.16/glib-String-Utility-Functions.html) (There may be a simpler way which someone else can suggest). Note: for future reference I suggest reviewing the doc/README.developer document (if you haven't already done so) which discusses such things as not using strcat & etc. 6. With respect to proto_reg_handoff: As you note in comment #7, the code involving tcp_port and global_tcp port is similar to that used in other dissectors. It needs one additional line to be OK: if (!initialized) { simulcrypt_handle = create_dissector_handle(dissect_simulcrypt, ... dissector_add("tcp.port", global_simulcrypt_tcp_port, ... initialized = TRUE; } --- The code which seems unneeded is: if(!strcmp(tab_ecm_inter[i].protocol_name,CA_SYSTEM_ID_MIKEY_PROTO)) { tab_ecm_inter[i].ca_system_id=ca_system_id_mikey; tab_ecm_inter[i].ecmg_port=-1; } datagram_handle=find_dissector(tab_ecm_inter[i].protocol_name); dissector_delete("tcp.port", tcp_port, datagram_handle); Let try to explain: a. As far as I can tell, The simulcrypt dissector never actually registers a handle for tab_ecm_inter[i].protocol_name on tcp_port and thus a dissector_delete is not required. (In actuality, altho the Wirehark documentation may not be clear, all that dissector delete really does is to deregister *whatever* handle is currently registered on the specified tcp port; the 'handle' argument is currently ignored). So: the find_dissector() ... delete_dissector lines are not required. b. ecmg_port is being used elsewhere to apparently determine when a sub-dissector should be called (find_dissector & etc). I would note that I think it's the case that find_dissector need be called only once during proto_reg_handoff for each of the protocols in tab_ecm_inter[] (with the handle being stored in another variable in tab_ecm_inter. I'm not sure when ecm_port needs to be initialized to -1; maybe for each new capture via an init function registered using 'register_init_routine' ? (The registered init function will be called each time a capture file dissection is done). Please let me know if I'm missing something bugzilla-daemon< at >wireshark.org 2008-10-07T18:15:25 http://permalink.gmane.org/gmane.network.wireshark.bugs/11140 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2913 David <david_aggeler< at >hispeed.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #2316|export_object_dicom.c |export_object_dicom.c diff description|comment spelling | bugzilla-daemon< at >wireshark.org 2008-10-07T17:23:29 http://permalink.gmane.org/gmane.network.wireshark.bugs/11139 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2913 David <david_aggeler< at >hispeed.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #2315|packet-dcm.h Corrected |packet-dcm.h diff description|comment | bugzilla-daemon< at >wireshark.org 2008-10-07T17:23:06 http://permalink.gmane.org/gmane.network.wireshark.bugs/11138 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2913 David <david_aggeler< at >hispeed.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #2314|packet-dcm.c Tag Support |packet-dcm.c diff description| | bugzilla-daemon< at >wireshark.org 2008-10-07T17:22:23 http://permalink.gmane.org/gmane.network.wireshark.bugs/11137 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2935 --- Comment #8 from Bill Meier <wmeier< at >newsguy.com> 2008-10-07 07:41:45 PDT --- It just so happens I've been looking at the original patch in more depth; I'll post some additional comments in an hour or so. To save you some effort, I suggest holding off on uploading & etc until you've had a chance to review my new comments. bugzilla-daemon< at >wireshark.org 2008-10-07T14:41:46 http://permalink.gmane.org/gmane.network.wireshark.bugs/11136 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2935 --- Comment #7 from David Castleford <david.castleford< at >orange-ftgroup.com> 2008-10-07 07:14:57 PDT --- (In reply to comment #5) OK OK Indeed, fixed, thanks. Correct, removed. Guess not, put things in .c only see void proto_reg_handoff_simulcrypt(void) that handles table registering ca_system_id value and corresponding ecmg (tcp) port used to update values when preferences are changed. If preferences are not initialized, default tcp port is set: (dissector_add("tcp.port", global_simulcrypt_tcp_port, simulcrypt_handle);) , otherwise dissector_delete the current port: dissector_delete("tcp.port", tcp_port, simulcrypt_handle); and dissector_add the port set in preferences: dissector_add("tcp.port", global_simulcrypt_tcp_port, simulcrypt_handle); I believe it works (I used example from existing dissectors). I have to check the dissector still compiles and works, I'll then make a diff file and upload to replace the initial version. Thanks for your help (all of you) bugzilla-daemon< at >wireshark.org 2008-10-07T14:14:58 http://permalink.gmane.org/gmane.network.wireshark.bugs/11135 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2935 --- Comment #6 from David Castleford <david.castleford< at >orange-ftgroup.com> 2008-10-07 06:59:38 PDT --- (In reply to comment #4) OK OK OK, added for messagetypenames, for parametertypenames value 0 is covered by SIMULCRYPT_DVB_RESERVED = 0x0000 OK, used NULL good catch, it's version, set to NULL now OK, 1 byte so set to 8 thanks, have done so, much nicer code. will reply to other comments, recompile and check things still work, create new diff file and upload patch bugzilla-daemon< at >wireshark.org 2008-10-07T13:59:40 http://permalink.gmane.org/gmane.network.wireshark.bugs/11134 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2944 Jaap Keuter <jaap.keuter< at >xs4all.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED --- Comment #2 from Jaap Keuter <jaap.keuter< at >xs4all.nl> 2008-10-07 00:30:06 PDT --- Fixed in revision 26369. bugzilla-daemon< at >wireshark.org 2008-10-07T07:30:07 http://permalink.gmane.org/gmane.network.wireshark.bugs/11133 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1967 --- Comment #7 from Jaap Keuter <jaap.keuter< at >xs4all.nl> 2008-10-07 00:05:28 PDT --- Frank, is there any news? Otherwise we're closing this bug. bugzilla-daemon< at >wireshark.org 2008-10-07T07:05:29 http://permalink.gmane.org/gmane.network.wireshark.bugs/11132 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2638 Jaap Keuter <jaap.keuter< at >xs4all.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED --- Comment #4 from Jaap Keuter <jaap.keuter< at >xs4all.nl> 2008-10-06 23:53:46 PDT --- Fixed in revision 26368. bugzilla-daemon< at >wireshark.org 2008-10-07T06:53:47 http://permalink.gmane.org/gmane.network.wireshark.bugs/11131 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2944 Wong Khai Thye <khaithye< at >yahoo.com.sg> changed: What |Removed |Added ---------------------------------------------------------------------------- OS/Version|Windows 2000 |All Platform|PC |All --- Comment #1 from Wong Khai Thye <khaithye< at >yahoo.com.sg> 2008-10-06 23:38:56 PDT --- Bug should be in "packet-gprs-llc.c". bugzilla-daemon< at >wireshark.org 2008-10-07T06:38:56 http://permalink.gmane.org/gmane.network.wireshark.bugs/11130 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2944 Summary: LLC GPRS - "Unnumbered Information" should be "Unconfirmed Information" Product: Wireshark Version: 1.0.3 Platform: PC OS/Version: Windows 2000 Status: NEW Severity: Minor Priority: Low Component: Wireshark AssignedTo: wireshark-bugs< at >wireshark.org ReportedBy: khaithye< at >yahoo.com.sg Created an attachment (id=2322) --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=2322) Example of a GPRS LLC UI frame Build Information: Shark 1.0.3 (SVN Rev 26134) Copyright 1998-2008 Gerald Combs <gerald< at >wireshark.org> and contributors. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Compiled with GLib 2.14.6, with WinPcap (version unknown), with libz 1.2.3, without POSIX capabilities, with libpcre 7.0, with SMI 0.4.8, with ADNS, with Lua 5.1, with GnuTLS 2.3.8, with Gcrypt 1.4.1, with MIT Kerberos. Running on Windows 2000 Service Pack 4, build 2195, with WinPcap version 4.0.2 (packet.dll version 4.0.0.1040), based on libpcap version 0.9.5. Built using Microsoft Visual C++ 6.0 build 8804 -- In the decoding of GPRS LLC layer, the UI format is erroneously decoded as "Unnumbered Information", but according to ETSI 04.64, it should be "Unconfirmed Information". The term "unnumbered" is misleading, because there is an N(U) in this layer for UI format (so, it's actually numbered). Following is an example: MS-SGSN LLC (Mobile Station - Serving GPRS Support Node Logical Link Control) SAPI: GPRS Mobility Management FCS: 0x21c14e (correct) Address field SAPI: LLGMM 0... .... = Protocol Discriminator_bit: OK .0.. .... = Command/Response bit: DownLink/UpLink = Response/Command .... 0001 = SAPI: GPRS Mobility Management (1) Unnumbered Information format - UI, N(U) = 0 110. .... .... .... = UI format: 0x0006 ...0 0... .... .... = Spare bits: 0x0000 .... .000 0000 00.. = N(U): 0 .... .... .... ..0. = E bit: non encrypted frame .... .... .... ...1 = PM bit: FCS covers the frame header and information fields bugzilla-daemon< at >wireshark.org 2008-10-07T06:36:40 http://permalink.gmane.org/gmane.network.wireshark.bugs/11129 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2855 Anders Broman <anders.broman< at >ericsson.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution| |FIXED --- Comment #4 from Anders Broman <anders.broman< at >ericsson.com> 2008-10-06 22:55:53 PDT --- Or should we include r25854 as well? "Log: From Vincent Helfre : GPRS LLC CRC24 badly calculated for non protected frames https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2290". I'll close it again as it's fixed in trunk any way. bugzilla-daemon< at >wireshark.org 2008-10-07T05:55:54 http://permalink.gmane.org/gmane.network.wireshark.bugs/11128 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2855 Gerald Combs <gerald< at >wireshark.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gerald< at >wireshark.org Status|RESOLVED |REOPENED Resolution|FIXED | --- Comment #3 from Gerald Combs <gerald< at >wireshark.org> 2008-10-06 14:10:54 PDT --- The fix for this patch (r26160) is on the pending list for 1.0.4. However, it appears to depend on r25854. Is this bug present in 1.0.3? If not, we can skip including r26160 in 1.0.4. bugzilla-daemon< at >wireshark.org 2008-10-06T21:10:54 http://permalink.gmane.org/gmane.network.wireshark.bugs/11127 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2631 Sven Eckelmann <sven.eckelmann< at >gmx.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #2298|review_for_checkin? | Flag| | Attachment #2298 is|0 |1 obsolete| | Attachment #2321| |review_for_checkin? Flag| | --- Comment #29 from Sven Eckelmann <sven.eckelmann< at >gmx.de> 2008-10-06 14:03:08 PDT --- Created an attachment (id=2321) --> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=2321) B.A.T.M.A.N Advanced dissector - Move proto_* functions to the end of packet-batadv.c - Fix patching against current trunk bugzilla-daemon< at >wireshark.org 2008-10-06T21:03:09 http://permalink.gmane.org/gmane.network.wireshark.bugs/11126 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2933 --- Comment #3 from Sven Eckelmann <sven.eckelmann< at >gmx.de> 2008-10-06 13:51:15 PDT --- Thx, looks fine bugzilla-daemon< at >wireshark.org 2008-10-06T20:51:15 http://permalink.gmane.org/gmane.network.wireshark.bugs/11125 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2631 Sven Eckelmann <sven.eckelmann< at >gmx.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED | --- Comment #28 from Sven Eckelmann <sven.eckelmann< at >gmx.de> 2008-10-06 13:41:04 PDT --- Sry, but you closed the wrong bug. r26362 belongs to bug 2933 bugzilla-daemon< at >wireshark.org 2008-10-06T20:41:05 http://permalink.gmane.org/gmane.network.wireshark.bugs/11124 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2631 Anders Broman <anders.broman< at >ericsson.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED --- Comment #27 from Anders Broman <anders.broman< at >ericsson.com> 2008-10-06 13:28:14 PDT --- Hopfully covered by revision 26362. If not a new patch against current SVN would be apreciated. bugzilla-daemon< at >wireshark.org 2008-10-06T20:28:15 http://permalink.gmane.org/gmane.network.wireshark.bugs/11123 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2933 Anders Broman <anders.broman< at >ericsson.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |anders.broman< at >ericsson.com Status|NEW |RESOLVED Resolution| |FIXED --- Comment #2 from Anders Broman <anders.broman< at >ericsson.com> 2008-10-06 13:26:13 PDT --- Committed revision 26362. Well a balance between many files and to large ones, that is. bugzilla-daemon< at >wireshark.org 2008-10-06T20:26:14 Search the mailing list at Gmane query http://search.gmane.org/?group=$group=gmane.network.wireshark.bugs