[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