[Pidgin] #5613: Problems detected using static analysis tool

Pidgin trac at pidgin.im
Fri Apr 25 10:17:39 EDT 2008


#5613: Problems detected using static analysis tool
---------------------------+------------------------------------------------
  Reporter:  julianday     |       Owner:  lschiere
      Type:  patch         |      Status:  new     
  Priority:  minor         |   Milestone:          
 Component:  unclassified  |     Version:  2.4.1   
Resolution:                |    Keywords:          
   Pending:  0             |  
---------------------------+------------------------------------------------
Comment (by julianday):

 Sorry, if I have made any mistakes, I love pidgin, but hate the crashes.
 I'm new to pidgin devel and I'm just trying to help. I'm working towards
 an updated patch addressing these things, but see below.

 Replying to [comment:6 sadrul]:
 > There's an incredible amount of unnecessary casting in the current
 patch. I think they should be removed (unless the compiler cries in their
 absence).
 There are a number of casts to enum typedefs from integer values. This is
 a warning identified in Annex J of the ANSI C standard. In the scope of a
 program the size of pidgin, I don't think that there were that many
 incidents of this, so it seems reasonable to address them.

 Another cast related change was using void* to indicate a function
 pointer, as in log.[ch]. This is a portability issue as identified in K5.7
 of the ANSI C standard. I've rejigged my changes in log.[ch], to something
 that should be more acceptable.

 > Some changes are also wrong (e.g. the first few blocks in blist.c).
 Sorry, my changes to purple_blist_add_buddy() were both wrong and strictly
 unnecesary. What happened here is the static analyser saw the NULL check
 in PURPLE_BUDDY_IS_ONLINE. It infers from that check that the pointer
 might be NULL. In the updated patch I'll submit (take it or leave it) the
 change now removes the duplicate checks.

 > There are also some unnecessary NULL checks (e.g. in myspace/message.c).
 Please remove those.
 Ok. These are actually due to duplicate NULL checks with the pointer
 dereferenced in-between, which the static analysis tool is complaining
 about. One in the exit condition for the loop and the other in the
 g_list_next macro.
 >
 > There seem to be some valid removal of redundant code. They should just
 be removed, instead of being wrapped inside '#if 0'.
 >
 Ok. I'll leave a comment saying it was redundant, just to clarify for
 whoever is merging. All my comments will be marked with JulianDay to make
 them easy to find.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/5613#comment:8>
Pidgin <http://pidgin.im>
Pidgin


More information about the Tracker mailing list