Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
On Fri, Apr 22, 2011 at 11:18:22AM -0400, Jeff Morriss wrote: > Stephen Fisher wrote: > > On Fri, Apr 22, 2011 at 03:09:01PM +0200, Alexis La Goutte wrote: > > > >> Why not add a check (in checkAPIs.pl) if proto_item_add_subtree always > >> return a value ? > > > > Good idea. Another way to do it is a gcc attribute called > > warn_unused_result: > > > > http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html > > That was my thought this morning as well; I added that check (after > fixing all the resulting warnings) in rev 36812. ... and Glib since 2.10 have got G_GNUC_WARN_UNUSED_RESULT [1] >From r36812: >> /* XXX where should this go? */ I thought about g_gnuc_malloc.h but name is little confusing. What about renaming g_gnuc_malloc.h to g_gnuc.h? [1] http://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#G-GNUC-WARN-UNUSED-RESULT:CAPS ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
On Fri, Apr 22, 2011 at 5:18 PM, Jeff Morriss wrote: > Stephen Fisher wrote: > >> On Fri, Apr 22, 2011 at 03:09:01PM +0200, Alexis La Goutte wrote: >> >> Why not add a check (in checkAPIs.pl) if proto_item_add_subtree always >>> return a value ? >>> >> >> Good idea. Another way to do it is a gcc attribute called >> warn_unused_result: >> http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html >> > > That was my thought this morning as well; I added that check (after fixing > all the resulting warnings) in rev 36812. > > Great :) ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
Stephen Fisher wrote: On Fri, Apr 22, 2011 at 03:09:01PM +0200, Alexis La Goutte wrote: Why not add a check (in checkAPIs.pl) if proto_item_add_subtree always return a value ? Good idea. Another way to do it is a gcc attribute called warn_unused_result: http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html That was my thought this morning as well; I added that check (after fixing all the resulting warnings) in rev 36812. ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
On Fri, Apr 22, 2011 at 03:09:01PM +0200, Alexis La Goutte wrote: > Why not add a check (in checkAPIs.pl) if proto_item_add_subtree always > return a value ? Good idea. Another way to do it is a gcc attribute called warn_unused_result: http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
2011/4/21 Stig Bjørlykke > On Thu, Apr 21, 2011 at 10:31 PM, Alexis La Goutte > wrote: > > I based my change on the previous revision of jmayer (rev36724) in this > file > > and there is the same mistake ! > > Hmm, after a closer look I find that proto_item_add_subtree() returns > the input parameter, so we have no real bug here. > Good to known ! > > But this raises a question why we have to use the return value from > proto_item_add_subtree() for the tree, as proto_item and proto_tree > are the same... I think the cleanest solution is to use the return > value, as this is done elsewhere and the implementation of > proto_item_add_subtree() may change. Comments? > Yes, it is better solution ! Fix in Rev 36801 Why not add a check (in checkAPIs.pl) if proto_item_add_subtree always return a value ? ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
Stig Bjørlykke wrote: On Thu, Apr 21, 2011 at 10:31 PM, Alexis La Goutte wrote: I based my change on the previous revision of jmayer (rev36724) in this file and there is the same mistake ! Hmm, after a closer look I find that proto_item_add_subtree() returns the input parameter, so we have no real bug here. But this raises a question why we have to use the return value from proto_item_add_subtree() for the tree, as proto_item and proto_tree are the same... I think the cleanest solution is to use the return value, as this is done elsewhere and the implementation of proto_item_add_subtree() may change. Comments? Isn't the theory that, while proto_item is currently the same as proto_tree, that could eventually (need to) change? ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
On Thu, Apr 21, 2011 at 10:31 PM, Alexis La Goutte wrote: > I based my change on the previous revision of jmayer (rev36724) in this file > and there is the same mistake ! Hmm, after a closer look I find that proto_item_add_subtree() returns the input parameter, so we have no real bug here. But this raises a question why we have to use the return value from proto_item_add_subtree() for the tree, as proto_item and proto_tree are the same... I think the cleanest solution is to use the return value, as this is done elsewhere and the implementation of proto_item_add_subtree() may change. Comments? -- Stig Bjørlykke ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
2011/4/21 Stig Bjørlykke > On Thu, Apr 21, 2011 at 6:01 PM, wrote: > > Fix Dead Store (Dead assignement/Dead increment) Warning found by Clang > > -flags1_tree = proto_item_add_subtree(flags1_item, > ett_pn_io_profisafe_f_parameter_prm_flag1); > +proto_item_add_subtree(flags1_item, > ett_pn_io_profisafe_f_parameter_prm_flag1); > > This fix is completely wrong! > > You have no need for a proto_item_add_subtree without using the return > value. > In this case I suppose flags1_tree should be use in the next > dissect_dcerpc_uint8 calls instead of using flags1_item. > > > Hi, I based my change on the previous revision of jmayer (rev36724) in this file ... and there is the same mistake ! I can not fix the issue tonight, I do it tomorrow Regards, ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
Re: [Wireshark-dev] [Wireshark-commits] rev 36772: /trunk/plugins/profinet/ /trunk/plugins/profinet/: packet-dcerpc-pn-io.c
On Thu, Apr 21, 2011 at 6:01 PM, wrote: > Fix Dead Store (Dead assignement/Dead increment) Warning found by Clang -flags1_tree = proto_item_add_subtree(flags1_item, ett_pn_io_profisafe_f_parameter_prm_flag1); +proto_item_add_subtree(flags1_item, ett_pn_io_profisafe_f_parameter_prm_flag1); This fix is completely wrong! You have no need for a proto_item_add_subtree without using the return value. In this case I suppose flags1_tree should be use in the next dissect_dcerpc_uint8 calls instead of using flags1_item. -- Stig Bjørlykke ___ Sent via:Wireshark-dev mailing list Archives:http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe
