> On March 31, 2015, 7:14 p.m., Mark Michelson wrote:
> > 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.
Ok you know that to prevent an unintendet assignment where a comparisson was
indended compilers request you to write
/* comparisson */
if (x == 1) {
}
but
/* assignment => most modern compilers will complain*/
if (x = 1) { /* <= warning raised*/
}
so you are forced to write
if ((x = 1)) {
}
instead, to guarantee that you meant to assign a value here.
But most compilers will not complain if you write the first comparison between
double parantheses
if ((x == 1)) {
}
Which is perfectly legal, as you pointed out.
But a maintainer of the code might later be in doubt as to what you mean. Was
an assignment was not intended by the original writer (non-obvious).
clang can be made complain about this (-Wparantheses-equalty or -Wall -Werror)
to make sure this last one is not allowed.
So this can be considered the reverse of the first warning, where an assignment
requires double parantheses.
- Diederik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4550/#review14987
-----------------------------------------------------------
On March 29, 2015, 7: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, 7: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
