[Pidgin] #7340: Bad programming practice

[Pidgin]

#7340: Bad programming practice
--------------------------+-------------------------------------------------
 Reporter:  fsilveira     |     Owner:  lschiere
     Type:  defect        |    Status:  new     
Component:  unclassified  |   Version:  2.5.2   
 Keywords:                |  
--------------------------+-------------------------------------------------
 While checking the changes between 2.5.1 and 2.5.2, I noted that there is
 a "#define foo(bar) { ... }". By some people (including myself) this is
 defined as a bad programming practice, as explained here[1] and here[2].

 While browsing the source code looking for examples of bad behavior, I
 found one located at "pidgin-2.5.2/libpurple/util.c:1457". The ALLOW_TAG
 is a macro that expands to two code blocks, and the second one will be out
 of the "if" block, opposed to what most could expect. I did not check if
 it is really wrong or right, but even if it is right new coders could
 misunderstand and make a mess. In my opinion this code should be reviewed.

 My initial patch to change all "#define foo(bar) { ... }" calls I could
 "grep" to "#define foo(bar) do { ... } while (0)" is attached. It compiles
 but is still untested.

 References:

 1. http://kernelnewbies.org/FAQ/DoWhile0

 2. http://www.google.com/search?q=define+do+while+(0)

-- 
Ticket URL: <http://developer.pidgin.im/ticket/7340>
Pidgin <http://pidgin.im>
Pidgin