-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4550/#review14987
-----------------------------------------------------------


I don't understand the purpose of this warning. I tried searching for details 
about this warning flag on the internet and came up empty, so I can't find any 
documentation that explains what type of error this check is supposed to help 
avoid.

It appears that the warning is intended to get rid of "extra" parentheses where 
they are unnecessary. The problem is that I don't see anything wrong with 
having them there, especially in macro definitions.

- Mark Michelson


On March 29, 2015, 5:14 p.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4550/
> -----------------------------------------------------------
> 
> (Updated March 29, 2015, 5:14 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24917
>     https://issues.asterisk.org/jira/browse/ASTERISK-24917
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> clang's static analyzer will throw quite a number warnings / errors during 
> compilation, some of which can be very helpfull in finding corner-case bugs.
> 
> clang compiler warning:--dev-mode and -Wparentheses-equality
> 
> include/asterisk/linkedlists.h:450
> ==================================
> Got rid of the extraneous "()" in the comparison to NULL by negating the 
> comparison
> 
> include/asterisk/vector.h:261
> =============================
> Removed the extraneous "()". Not particularly happy about this though as they 
> where used to keep this macro encapsulated (Does however not cause any 
> compile issues)
> 
> Another possible solutions would be to double negate the comparison !((elem) 
> != (value)) which would keep everything encapsulated, but does result in a 
> double negative, which is ugly as well.
> 
> main/format_cap.c:467
> =====================
> Removed the extraneous "()". Not particularly happy about this though as they 
> where used to keep this macro encapsulated (Does however not cause any 
> compile issues)
> 
> Another possible solutions would be to double negate the comparison 
> !((elem)->format != (value)) which would keep everything encapsulated, but 
> does result in a double negative, which is ugly as well.
> 
> main/format_cap.c:492
> =====================
> Because of the () where removed previously, they are now needed here.
> 
> main/stasis_message_router.c:82
> ===============================
> Removed the extraneous "()". Not particularly happy about this though as they 
> where used to keep this macro encapsulated (Does however not cause any 
> compile issues)
> 
> Another possible solutions would be to double negate the comparison 
> !((elem).message_type != (value)) which would keep everything encapsulated, 
> but does result in a double negative, which is ugly as well.
> 
> main/stdtime/localtime.c:197/208
> ================================
> Removed the extraneous "()". Not particularly happy about this though as they 
> where used to keep this macro encapsulated (Does however not cause any 
> compile issues)
> 
> Another possible solutions would be to double negate the comparison 
> !((sp)->wd[0] != SP_STACK_FLAG) which would keep everything encapsulated, but 
> does result in a double negative, which is ugly as well.
> 
> more of the same for
> ====================
> include/asterisk/dlinkedlists.h:422/431/469
> main/astobj2_hash.c:768
> 
> ---------
> Maybe one of you has a better/nicer solution.
> 
> 
> Diffs
> -----
> 
>   /branches/13/main/stdtime/localtime.c 433444 
>   /branches/13/main/stasis_message_router.c 433444 
>   /branches/13/main/format_cap.c 433444 
>   /branches/13/main/astobj2_hash.c 433444 
>   /branches/13/include/asterisk/vector.h 433444 
>   /branches/13/include/asterisk/linkedlists.h 433444 
>   /branches/13/include/asterisk/dlinkedlists.h 433444 
> 
> Diff: https://reviewboard.asterisk.org/r/4550/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to