QA-C gives side-effects warning. Unknown why

Discussion in 'C Programming' started by Harry Ebbers, Nov 26, 2008.

  1. Harry Ebbers

    Harry Ebbers Guest

    Hi,

    For some piece of code (an if-statement with multiple function-calls
    to the same get-function in its condition) QA-C gives the following
    warning on the 2 and further times the function is called:
    "The right operand of '&&', '||', '?' or ':' has side effects, and
    will only be executed under certain conditions. This is unclear. Try
    using an explicit condition."
    According to our coding-standard we should solve this QA-C warning.
    But we want to understand first why this warning occurs.

    And there lies the problem. We are breaking our head over why we get
    this warning, since the function it calls is a get-function which does
    not alterate any values. Note: the get-function is located in a
    different module file than the if-statement.


    The code looks like this:
    (Note: SOME_TYPE is an enum-type)

    Module_A.c

    static SOME_TYPE some_arr[SIZE];

    extern SOME_TYPE get_value(int index)
    {
    SOME_TYPE status = SOME_VALUE_0;

    status = some_arr[index];

    return status;
    }

    Module_B.c

    #include <Module_A.h>

    void function_with_QA_C_warning(void)
    {
    ....
    ....
    /* The lines marked with # are the lines where QA-C gives the
    warning on */
    if ((get_value(index1) == SOME_VALUE_1)
    # || (get_value(index2) == SOME_VALUE_1)
    # || (get_value(index3) == SOME_VALUE_1)
    # || (get_value(index4) == SOME_VALUE_1))
    {
    ....
    }
    ....
    }

    Can anybody explain to us why QA-C sees these function-calls as calls
    with side-effects?

    Kind regards,

    Harry Ebbers
     
    Harry Ebbers, Nov 26, 2008
    #1
    1. Advertising

  2. Harry Ebbers

    Willem Guest

    Harry Ebbers wrote:
    ) Hi,
    )
    ) For some piece of code (an if-statement with multiple function-calls
    ) to the same get-function in its condition) QA-C gives the following
    ) warning on the 2 and further times the function is called:
    ) "The right operand of '&&', '||', '?' or ':' has side effects, and
    ) will only be executed under certain conditions. This is unclear. Try
    ) using an explicit condition."

    It doesn't say that the *function* has side effects, it tells you that the
    *operand* has side effects. The operand, in this case, contains a function
    call and in the mind of the compiler, function call implies side effect.

    ) Can anybody explain to us why QA-C sees these function-calls as calls
    ) with side-effects?

    The compiler probably assumes that any function call has side effects.

    Perhaps you can convince the compiler that that function does not have
    side effects, with some compiler-specific extension.


    SaSW, Willem
    --
    Disclaimer: I am in no way responsible for any of the statements
    made in the above text. For all I know I might be
    drugged or something..
    No I'm not paranoid. You all think I'm paranoid, don't you !
    #EOT
     
    Willem, Nov 26, 2008
    #2
    1. Advertising

  3. Harry Ebbers

    Chris Dollin Guest

    Harry Ebbers wrote:

    > Can anybody explain to us why QA-C sees these function-calls as calls
    > with side-effects?


    It's guessing. (Either that, or it does some clever analysis but gets it
    wrong, or right but not useful; you might want to double-check the code
    for `get_value` in case it does something like logging, a strangely
    popular side-effect).

    My advice: find out how to switch off the warning. (Perhaps there's a
    way of annotating `get_value` with "No side effects here, honest, guv.".)
    A C programmer that /doesn't/ know, and indeed exploit, that the right
    operand of `||` will only be evaluated if the left operand is false, is
    a C programmer with a learning opportunity.

    If you can't switch the warning off, work out how you can filter it.

    If you can't filter it, change compilers or change coding standards,
    whichever is easier.

    --
    "It took a very long time, much longer than the most /Sector General/
    generous estimates." - James White

    Hewlett-Packard Limited registered no:
    registered office: Cain Road, Bracknell, Berks RG12 1HN 690597 England
     
    Chris Dollin, Nov 26, 2008
    #3
  4. Harry Ebbers

    Chris Dollin Guest

    Harry Ebbers wrote:

    > The code looks like this:
    > (Note: SOME_TYPE is an enum-type)
    >
    > Module_A.c
    >
    > static SOME_TYPE some_arr[SIZE];
    >
    > extern SOME_TYPE get_value(int index)
    > {
    > SOME_TYPE status = SOME_VALUE_0;
    >
    > status = some_arr[index];
    >
    > return status;
    > }


    How much like that does it look? Because if it looks a lot
    like that, why doesn't it look more like

    return some_arr[index];

    ? Initialising a status variable in its declaration, then
    unconditionally assigning it a new value, and then returning
    the value of the variable, ie the assigned value, is a long
    way round the houses.

    Even if you check that `index` is sane first, I can't think of a
    good reason for the code to limp along like that. (Perhaps it
    doesn't really look much like the code above.)

    --
    "Look at the day that is dawning" - Caravan, /Nine Feet Underground/

    Hewlett-Packard Limited Cain Road, Bracknell, registered no:
    registered office: Berks RG12 1HN 690597 England
     
    Chris Dollin, Nov 26, 2008
    #4
  5. Harry Ebbers

    Harry Ebbers Guest

    On Nov 26, 11:47 am, Chris Dollin <> wrote:
    > Harry Ebbers wrote:
    > > The code looks like this:
    > > (Note: SOME_TYPE is an enum-type)

    >
    > > Module_A.c

    >
    > > static SOME_TYPE some_arr[SIZE];

    >
    > > extern SOME_TYPE get_value(int index)
    > > {
    > > SOME_TYPE status = SOME_VALUE_0;

    >
    > > status = some_arr[index];

    >
    > > return status;
    > > }

    >
    > How much like that does it look?

    It actually looks a bit more like:

    Module_A.c

    static SOME_TYPE some_arr[SIZE];

    extern SOME_TYPE get_value(int index)
    {
    const char *func_name = "get_value";
    SOME_TYPE status = SOME_VALUE_0;

    status = some_arr[index];

    Trace("<component_name>", INTERNAL, func_name, " index: %d, status:
    %s", index, SOME_TYPE_2_STR(status));

    return status;
    }

    Where Trace() is a function that sends some logging to a tracing-
    process.

    Thus there is some logging involved, which can be the cause of the
    problem if I read your other post correctly.
    I simpliefied it a bit, since other functions which are used in a
    similar if-construction and which also use the Trace() function don't
    get the QA-C warning.

    Kind regards,

    Harry
     
    Harry Ebbers, Nov 26, 2008
    #5
  6. Harry Ebbers

    Willem Guest

    Harry Ebbers wrote:
    ) It actually looks a bit more like:
    )
    ) Module_A.c
    )
    ) static SOME_TYPE some_arr[SIZE];
    )
    ) extern SOME_TYPE get_value(int index)
    ) {
    ) const char *func_name = "get_value";
    ) SOME_TYPE status = SOME_VALUE_0;
    )
    ) status = some_arr[index];
    )
    ) Trace("<component_name>", INTERNAL, func_name, " index: %d, status:
    ) %s", index, SOME_TYPE_2_STR(status));
    )
    ) return status;
    ) }

    Those look a hell of a lot like side effects to me.

    But in any case, side effects occurring in short-circuited AND and OR
    constructions are a often-used C idiom. There may be reasons for the
    compiler to mention them, but insisting that every single such warning
    is taken care of is ridiculous(1).
    Perhaps there is a compiler-specific way to make it shut up about it.

    ) Thus there is some logging involved, which can be the cause of the
    ) problem if I read your other post correctly.
    ) I simpliefied it a bit, since other functions which are used in a
    ) similar if-construction and which also use the Trace() function don't
    ) get the QA-C warning.

    I find that very strange, because logging is a definite side effect.


    1) A personal gripe of mine is C code littered with '(void)' casts
    meant to suppress the 'unused return value' warnings.


    SaSW, Willem
    --
    Disclaimer: I am in no way responsible for any of the statements
    made in the above text. For all I know I might be
    drugged or something..
    No I'm not paranoid. You all think I'm paranoid, don't you !
    #EOT
     
    Willem, Nov 26, 2008
    #6
  7. Harry Ebbers

    James Kuyper Guest

    Harry Ebbers wrote:
    > Hi,
    >
    > For some piece of code (an if-statement with multiple function-calls
    > to the same get-function in its condition) QA-C gives the following
    > warning on the 2 and further times the function is called:
    > "The right operand of '&&', '||', '?' or ':' has side effects, and
    > will only be executed under certain conditions. This is unclear. Try
    > using an explicit condition."
    > According to our coding-standard we should solve this QA-C warning.
    > But we want to understand first why this warning occurs.
    >
    > And there lies the problem. We are breaking our head over why we get
    > this warning, since the function it calls is a get-function which does
    > not alterate any values. Note: the get-function is located in a
    > different module file than the if-statement.


    QA-C is a code checker that checks one module at a time. As a result,
    while analyzing this code, it can't know whether or not get_value() has
    any side-effects. The most conservative choice is to assume that
    get_value() might have side effects.

    > /* The lines marked with # are the lines where QA-C gives the
    > warning on */
    > if ((get_value(index1) == SOME_VALUE_1)
    > # || (get_value(index2) == SOME_VALUE_1)
    > # || (get_value(index3) == SOME_VALUE_1)
    > # || (get_value(index4) == SOME_VALUE_1))


    I used to have access to QA-C. The coding standards it enforces are
    highly configurable; this warning can probably be turned off. However,
    if your coding standard mandates that this particular feature of QA-C be
    turned on, I can see the point. If the compiler can't be sure that
    get_value() has no side-effects without looking outside of this module,
    than neither can you.

    If the contents of get_value() are simple enough, you might try
    declaring it as 'static inline' and including the complete function
    definition in the appropriate header file. Since I no longer have access
    to QA-C, I can't verify this, but it seems plausible.
     
    James Kuyper, Nov 26, 2008
    #7
  8. Harry Ebbers

    Harry Ebbers Guest

    On Nov 26, 1:12 pm, Willem <> wrote:
    > Harry Ebbers wrote:
    >
    > ) It actually looks a bit more like:
    > )
    > ) Module_A.c
    > )
    > ) static SOME_TYPE some_arr[SIZE];
    > )
    > ) extern SOME_TYPE get_value(int index)
    > ) {
    > ) const char *func_name = "get_value";
    > ) SOME_TYPE status = SOME_VALUE_0;
    > )
    > ) status = some_arr[index];
    > )
    > ) Trace("<component_name>", INTERNAL, func_name, " index: %d, status:
    > ) %s", index, SOME_TYPE_2_STR(status));
    > )
    > ) return status;
    > ) }
    >
    > Those look a hell of a lot like side effects to me.

    I checked if this Trace() function was the cause by commenting it out.
    But the QA-C warnings still occured.

    > But in any case, side effects occurring in short-circuited AND and OR
    > constructions are a often-used C idiom. There may be reasons for the
    > compiler to mention them, but insisting that every single such warning
    > is taken care of is ridiculous(1).

    That is why we want to understand why it is happening. "Don't fix it,
    if it ain't broken" is our motto (I'm working on a sustaining
    project).
    But we have to take a look at it when there is an issue with the
    coding-standard, since sometimes such a QA-C warning is showing a real
    error (especially with the higher level warnings, and this is a level
    4 one (lowest level that we have to fix)).
    And when there is a reason to leave it in, an explanatory comment in
    the code is also a "fix", since than people know why it is left in.

    > Perhaps there is a compiler-specific way to make it shut up about it.

    Also here our motto applies.

    > ) Thus there is some logging involved, which can be the cause of the
    > ) problem if I read your other post correctly.
    > ) I simpliefied it a bit, since other functions which are used in a
    > ) similar if-construction and which also use the Trace() function don't
    > ) get the QA-C warning.
    >
    > I find that very strange, because logging is a definite side effect.

    Seemingly it is not the cause of the side-effects (at least not
    according to my experiment with commenting it out).

    > 1) A personal gripe of mine is C code littered with '(void)' casts
    > meant to suppress the 'unused return value' warnings.

    Indeed a nice way to introduce bugs, in the situation that a function
    returned an error, which is not handled since it is cast to (void). >-
    (

    Kind regards,

    Harry

    > SaSW, Willem
    > --
    > Disclaimer: I am in no way responsible for any of the statements
    > made in the above text. For all I know I might be
    > drugged or something..
    > No I'm not paranoid. You all think I'm paranoid, don't you !
    > #EOT
     
    Harry Ebbers, Nov 26, 2008
    #8
  9. Harry Ebbers

    Stuart Guest

    "Harry Ebbers" <> wrote in message
    news:...

    > For some piece of code (an if-statement with multiple function-calls
    > to the same get-function in its condition) QA-C gives the following
    > warning on the 2 and further times the function is called:
    > "The right operand of '&&', '||', '?' or ':' has side effects, and
    > will only be executed under certain conditions. This is unclear. Try
    > using an explicit condition."

    ....
    > We are breaking our head over why we get
    > this warning, since the function it calls is a get-function which does
    > not alterate any values. Note: the get-function is located in a
    > different module file than the if-statement.


    I don't know QA-C or the depth to which it does this type of analysis (my
    experience is with Ada/SPARK).

    t would not be surprising if the get-function is modifying an input stream
    and this is the side effect that is being referred to. In this case the
    warning is that it is 'unclear' as to what state the input stream will end
    up being.

    I believe the reason it is labelled as being 'unclear' is because it is not
    certain whether you appreciated the existence of the side-effect and the
    consequences of '||'. If it had been written long-hand as a nested series
    of 'ifs' the assumption would [probably] be that you were aware of the
    side-effects.

    Regards
    Stuart
     
    Stuart, Nov 26, 2008
    #9
  10. Harry Ebbers

    James Kuyper Guest

    Harry Ebbers wrote:
    > On Nov 26, 1:12 pm, Willem <> wrote:
    >> Harry Ebbers wrote:

    ....
    >> ) Thus there is some logging involved, which can be the cause of the
    >> ) problem if I read your other post correctly.
    >> ) I simpliefied it a bit, since other functions which are used in a
    >> ) similar if-construction and which also use the Trace() function don't
    >> ) get the QA-C warning.
    >>
    >> I find that very strange, because logging is a definite side effect.

    > Seemingly it is not the cause of the side-effects (at least not
    > according to my experiment with commenting it out).


    What you've proven is that the logging code is not the cause of the
    warnings about side-effects. That's not the same a proving that it isn't
    a side effect.

    The C standard says:
    "Accessing a volatile object, modifying an object, modifying a file, or
    calling a function that does any of those operations are all _side
    effects_." (5.1.2.3p2). I use the underscores to indicate that "side
    effects" is in italic type. That's the standard's way of indicating that
    this sentence constitutes the definition of a side effect. Writing to a
    log file is definitely "modifying a file", and therefore unambiguously a
    side effect.

    Commenting out the logging code didn't have any effect on the warnings
    you got, because those warnings were not based upon an an analysis of
    the code for get_value(). Those warnings were based upon a worst-case
    analysis that assumed, since QA-C didn't know what get_value() does,
    that the function call might be a side effect.
     
    James Kuyper, Nov 26, 2008
    #10
  11. Harry Ebbers

    Chris Dollin Guest

    Stuart wrote:

    > "Harry Ebbers" <> wrote in message
    > news:...
    >
    >> For some piece of code (an if-statement with multiple function-calls
    >> to the same get-function in its condition) QA-C gives the following
    >> warning on the 2 and further times the function is called:
    >> "The right operand of '&&', '||', '?' or ':' has side effects, and
    >> will only be executed under certain conditions. This is unclear. Try
    >> using an explicit condition."


    > I believe the reason it is labelled as being 'unclear' is because it is not
    > certain whether you appreciated the existence of the side-effect and the
    > consequences of '||'.


    I think the reason it is labelled "unclear" is that the writers of the message
    were pessimistic about the quality of the programmers who would be asking
    for the warnings. (I don't know whether their pessimism is justified.)

    The behaviour of `a || b || ... || z` is perfectly clear once you've learned
    it, just like the behaviour of `=`, `while`, and `+`. I'll grant that the
    behaviour of `a || b && c` is less clear, but that wasn't the code that
    was being diagnosed.

    --
    "Look at the day that is dawning" - Caravan, /Nine Feet Underground/

    Hewlett-Packard Limited Cain Road, Bracknell, registered no:
    registered office: Berks RG12 1HN 690597 England
     
    Chris Dollin, Nov 26, 2008
    #11
  12. Harry Ebbers

    Harry Ebbers Guest

    On Nov 26, 2:12 pm, James Kuyper <> wrote:
    >
    > What you've proven is that the logging code is not the cause of the
    > warnings about side-effects. That's not the same a proving that it isn't
    > a side effect.

    You're right. I should have phrased that better.
    I was trying to rule it out as cause for the observed QA-C warnings
    and succeeded in that, but in nothing more.

    I did another experiment by copying the function to the module on
    which QA-C gave the warnings (and using that local version of the
    function i.s.o. the original function), just to check if that would
    solve it.
    But alas, the warnings stayed.

    Anyway in the meantime we have decided to leave it in for now and
    we've sent a mail to the deparment who made our coding standard, to
    see if they will come up with a solution. (E.g. change the error-text
    from "has side-effects" to "may have side-effects" and some
    clarification that this is also true for *any* function implemented in
    a different module).

    (A "solution" in this case would have been: assign the results of the
    function to some temp variables and use those variables in the if, but
    that sounds more like tool-satisfaction and therefore we do/did not
    want to do that (yet). (Better a real solution than just doing
    something just to satify a tool IMHO))

    Kind regards,

    Harry
     
    Harry Ebbers, Nov 26, 2008
    #12
  13. Harry Ebbers

    CBFalconer Guest

    Jack Klein wrote:
    > Harry Ebbers <> wrote in comp.lang.c:
    >
    >> For some piece of code (an if-statement with multiple function-
    >> calls to the same get-function in its condition) QA-C gives the
    >> following warning on the 2 and further times the function is
    >> called:
    >> "The right operand of '&&', '||', '?' or ':' has side effects,
    >> and will only be executed under certain conditions. This is
    >> unclear. Try using an explicit condition."
    >> According to our coding-standard we should solve this QA-C
    >> warning. But we want to understand first why this warning occurs.

    .... snip ...
    >> ....
    >> /* The lines marked with # are the lines where QA-C gives the
    >> warning on */
    >> if ((get_value(index1) == SOME_VALUE_1)
    >> # || (get_value(index2) == SOME_VALUE_1)
    >> # || (get_value(index3) == SOME_VALUE_1)
    >> # || (get_value(index4) == SOME_VALUE_1))
    >> {
    >>
    >> Can anybody explain to us why QA-C sees these function-calls as
    >> calls with side-effects?

    >
    > You will get a similar warning from PC Lint, for the same reason.
    > The analysis tool assumes that the function has side effects, and
    > depending on run time values, you may call it one, two, three, or
    > four times.
    >
    > With PC Lint, you could inform the tool that the function
    > get_value() does not have side effects. It's been quite a while
    > since I used QA-C, but most likely they have something similar.
    >
    > Errors with short-circuited side effects due to || and && do
    > happen, although they are not terribly common in my experience.
    > So the warning is not without value.


    I think the point being missed by the OP, and not really being made
    by the answers, is that the '||' code evaluates the operand, and
    avoids the rest of the statement if the answer is true. Thus, in
    the above, if "(get_value(index1) == SOME_VALUE_1)" returns a true
    all the other get_values will not be executed. If the OP wants to
    execute all those get_value calls, he should replace '||' with
    '|'. As it is, the first one to return a true settles the
    condition being tested by the if.

    I think the error message returned is confusing to the user.

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.
     
    CBFalconer, Nov 27, 2008
    #13
  14. Harry Ebbers

    Thad Smith Guest

    Harry Ebbers wrote:
    > Hi,
    >
    > For some piece of code (an if-statement with multiple function-calls
    > to the same get-function in its condition) QA-C gives the following
    > warning on the 2 and further times the function is called:
    > "The right operand of '&&', '||', '?' or ':' has side effects, and
    > will only be executed under certain conditions. This is unclear. Try
    > using an explicit condition."
    > According to our coding-standard we should solve this QA-C warning.

    ....
    > void function_with_QA_C_warning(void)
    > {
    > ....
    > ....
    > /* The lines marked with # are the lines where QA-C gives the
    > warning on */
    > if ((get_value(index1) == SOME_VALUE_1)
    > # || (get_value(index2) == SOME_VALUE_1)
    > # || (get_value(index3) == SOME_VALUE_1)
    > # || (get_value(index4) == SOME_VALUE_1))
    > {
    > ....
    > }
    > ....
    > }


    There are two ways I can think of that would probably avoid the warning:

    if ((get_value(index1) == SOME_VALUE_1)
    | (get_value(index2) == SOME_VALUE_1)
    | (get_value(index3) == SOME_VALUE_1)
    | (get_value(index4) == SOME_VALUE_1))
    {
    ....
    }

    That will work, but always calls get_value 4 times, rather than stopping
    with the first match of SOME_VALUE_1.

    The other is

    if (get_value(index1) != SOME_VALUE_1);
    else if (get_value(index2) != SOME_VALUE_1);
    else if (get_value(index3) != SOME_VALUE_1);
    else if (get_value(index4) != SOME_VALUE_1);
    else
    {
    ....
    }

    That is harder for me to read. I prefer your original code.

    --
    Thad
     
    Thad Smith, Nov 27, 2008
    #14
  15. Harry Ebbers

    Thad Smith Guest

    Jack Klein wrote:
    > On Wed, 26 Nov 2008 02:29:30 -0800 (PST), Harry Ebbers
    > <> wrote in comp.lang.c:
    >
    >> For some piece of code (an if-statement with multiple function-calls
    >> to the same get-function in its condition) QA-C gives the following
    >> warning on the 2 and further times the function is called:
    >> "The right operand of '&&', '||', '?' or ':' has side effects, and
    >> will only be executed under certain conditions. This is unclear. Try
    >> using an explicit condition."
    >> According to our coding-standard we should solve this QA-C warning.
    >> But we want to understand first why this warning occurs.


    > Errors with short-circuited side effects due to || and && do happen,
    > although they are not terribly common in my experience. So the
    > warning is not without value.


    Yes, the warnings have value, but once a warned item is examined and
    found benign, it can be used. I believe PC-Lint supports commands added
    in a comment to disable the warning locally.

    A coding standard, especially one with no easy means of accepting warned
    but validated code, can force the programmer into less-readable or
    less-efficient work arounds.

    --
    Thad
     
    Thad Smith, Nov 27, 2008
    #15
  16. CBFalconer <> writes:
    > Jack Klein wrote:
    >> Harry Ebbers <> wrote in comp.lang.c:
    >>
    >>> For some piece of code (an if-statement with multiple function-
    >>> calls to the same get-function in its condition) QA-C gives the
    >>> following warning on the 2 and further times the function is
    >>> called:
    >>> "The right operand of '&&', '||', '?' or ':' has side effects,
    >>> and will only be executed under certain conditions. This is
    >>> unclear. Try using an explicit condition."
    >>> According to our coding-standard we should solve this QA-C
    >>> warning. But we want to understand first why this warning occurs.

    > ... snip ...
    >>> ....
    >>> /* The lines marked with # are the lines where QA-C gives the
    >>> warning on */
    >>> if ((get_value(index1) == SOME_VALUE_1)
    >>> # || (get_value(index2) == SOME_VALUE_1)
    >>> # || (get_value(index3) == SOME_VALUE_1)
    >>> # || (get_value(index4) == SOME_VALUE_1))
    >>> {

    [...]
    > I think the point being missed by the OP, and not really being made
    > by the answers, is that the '||' code evaluates the operand, and
    > avoids the rest of the statement if the answer is true.


    Um, I don't think anybody missed that. (And it avoids the rest of the
    expression, not the rest of the statement.)

    > Thus, in
    > the above, if "(get_value(index1) == SOME_VALUE_1)" returns a true
    > all the other get_values will not be executed. If the OP wants to
    > execute all those get_value calls, he should replace '||' with
    > '|'. As it is, the first one to return a true settles the
    > condition being tested by the if.


    Replacing "||" by "|" seems like a really bad idea, just as a matter
    of style if nothing else. Even ignoring the short-circuit behavior,
    they mean different things; "||" is a logical or, and "|" is a bitwise
    or.

    Now it happens that all the operands are "==" comparisons, which are
    guaranteed to yield either 0 or 1, so the result will be the same *in
    this case*. But future maintenance could easily replace one of the
    expressions with a function call or variable reference that can use a
    non-zero value other than 1 to represent a true result. And anyone
    reading the code would have to go through the same reasoning to
    understand it -- or might assume that "|" is a typo for "||" and "fix"
    it.

    If I wanted to avoid the warning and guarantee that all the
    subexpressions are evaluated, I'd much rather write something like
    this:

    const int result1 = get_value(index1) == SOME_VALUE_1;
    const int result2 = get_value(index2) == SOME_VALUE_2;
    const int result3 = get_value(index3) == SOME_VALUE_3;
    const int result4 = get_value(index4) == SOME_VALUE_4;

    if (result1 || result2 || result3 || result4) {
    /* ... */
    }

    Depending on the code, assigning names to the subexpressions might
    even make it clearer.

    (I *might* consider writing a macro like:
    #define OR4(x1, x2, x3, x4) (!!(x1) | !!(x2) | !!(x3) | !!(x4))
    but I wouldn't consider it for very long.)

    The OP mentioned elsethread that get_value() writes trace messages to
    a log somewhere, so changing the code so it's always called 4 times
    really does change the behavior. He has to decide whether that
    matters.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Nov 27, 2008
    #16
  17. Thad Smith <> writes:

    > Harry Ebbers wrote:

    <snip>
    >> /* The lines marked with # are the lines where QA-C gives the
    >> warning on */
    >> if ((get_value(index1) == SOME_VALUE_1)
    >> # || (get_value(index2) == SOME_VALUE_1)
    >> # || (get_value(index3) == SOME_VALUE_1)
    >> # || (get_value(index4) == SOME_VALUE_1))
    >> {
    >> ....
    >> }
    >> ....
    >> }

    >
    > There are two ways I can think of that would probably avoid the warning:
    >
    > if ((get_value(index1) == SOME_VALUE_1)
    > | (get_value(index2) == SOME_VALUE_1)
    > | (get_value(index3) == SOME_VALUE_1)
    > | (get_value(index4) == SOME_VALUE_1))
    > {
    > ....
    > }
    >
    > That will work, but always calls get_value 4 times, rather than
    > stopping with the first match of SOME_VALUE_1.
    >
    > The other is
    >
    > if (get_value(index1) != SOME_VALUE_1);
    > else if (get_value(index2) != SOME_VALUE_1);
    > else if (get_value(index3) != SOME_VALUE_1);
    > else if (get_value(index4) != SOME_VALUE_1);
    > else
    > {
    > ....
    > }
    >
    > That is harder for me to read. I prefer your original code.


    Your second re-write is not the same thing at all! It is a form of

    if ((get_value(index1) == SOME_VALUE_1)
    && (get_value(index2) == SOME_VALUE_1)
    && (get_value(index3) == SOME_VALUE_1)
    && (get_value(index4) == SOME_VALUE_1))
    {
    ....
    }

    --
    Ben.
     
    Ben Bacarisse, Nov 27, 2008
    #17
  18. Harry Ebbers

    Harry Ebbers Guest

    On Nov 27, 3:03 am, CBFalconer <> wrote:
    > Jack Klein wrote:
    > > Harry Ebbers <> wrote in comp.lang.c:

    >
    > >> For some piece of code (an if-statement with multiple function-
    > >> calls to the same get-function in its condition) QA-C gives the
    > >> following warning on the 2 and further times the function is
    > >> called:
    > >> "The right operand of '&&', '||', '?' or ':' has side effects,
    > >> and will only be executed under certain conditions. This is
    > >> unclear. Try using an explicit condition."
    > >> According to our coding-standard we should solve this QA-C
    > >> warning. But we want to understand first why this warning occurs.

    > ... snip ...
    > >> ....
    > >> /* The lines marked with # are the lines where QA-C gives the
    > >> warning on */
    > >> if ((get_value(index1) == SOME_VALUE_1)
    > >> # || (get_value(index2) == SOME_VALUE_1)
    > >> # || (get_value(index3) == SOME_VALUE_1)
    > >> # || (get_value(index4) == SOME_VALUE_1))
    > >> {

    >
    > >> Can anybody explain to us why QA-C sees these function-calls as
    > >> calls with side-effects?

    >
    > > You will get a similar warning from PC Lint, for the same reason.
    > > The analysis tool assumes that the function has side effects, and
    > > depending on run time values, you may call it one, two, three, or
    > > four times.

    >
    > > With PC Lint, you could inform the tool that the function
    > > get_value() does not have side effects. It's been quite a while
    > > since I used QA-C, but most likely they have something similar.

    >
    > > Errors with short-circuited side effects due to || and && do
    > > happen, although they are not terribly common in my experience.
    > > So the warning is not without value.

    >
    > I think the point being missed by the OP, and not really being made
    > by the answers, is that the '||' code evaluates the operand, and
    > avoids the rest of the statement if the answer is true. Thus, in
    > the above, if "(get_value(index1) == SOME_VALUE_1)" returns a true
    > all the other get_values will not be executed. If the OP wants to
    > execute all those get_value calls, he should replace '||' with
    > '|'. As it is, the first one to return a true settles the
    > condition being tested by the if.
    >
    > I think the error message returned is confusing to the user.

    You are wrong. I did not miss this point.

    It is very clear to me that on an && the rest of the condition is not
    evaluated when the first part is FALSE, since the result of the &&
    will be FALSE anyway and with an || the rest is not evaluated when the
    first part is TRUE since the result off the || will be TRUE anyway.

    Since it was clear to me, I did not mention it in my original post,
    there was no need to.

    Please do not make assumptions, but keep to facts. In doubt ask, but
    don't assume.

    Kind regards,

    Harry
     
    Harry Ebbers, Nov 27, 2008
    #18
  19. Harry Ebbers

    Harry Ebbers Guest

    On Nov 27, 5:04 am, Ben Bacarisse <> wrote:
    > Thad Smith <> writes:
    > > Harry Ebbers wrote:

    > <snip>
    > >> /* The lines marked with # are the lines where QA-C gives the
    > >> warning on */
    > >> if ((get_value(index1) == SOME_VALUE_1)
    > >> # || (get_value(index2) == SOME_VALUE_1)
    > >> # || (get_value(index3) == SOME_VALUE_1)
    > >> # || (get_value(index4) == SOME_VALUE_1))
    > >> {
    > >> ....
    > >> }
    > >> ....
    > >> }

    >
    > > There are two ways I can think of that would probably avoid the warning:

    >
    > > if ((get_value(index1) == SOME_VALUE_1)
    > > | (get_value(index2) == SOME_VALUE_1)
    > > | (get_value(index3) == SOME_VALUE_1)
    > > | (get_value(index4) == SOME_VALUE_1))
    > > {
    > > ....
    > > }

    >
    > > That will work, but always calls get_value 4 times, rather than
    > > stopping with the first match of SOME_VALUE_1.

    >
    > > The other is

    >
    > > if (get_value(index1) != SOME_VALUE_1);
    > > else if (get_value(index2) != SOME_VALUE_1);
    > > else if (get_value(index3) != SOME_VALUE_1);
    > > else if (get_value(index4) != SOME_VALUE_1);
    > > else
    > > {
    > > ....
    > > }

    >
    > > That is harder for me to read. I prefer your original code.

    >
    > Your second re-write is not the same thing at all! It is a form of
    >
    > if ((get_value(index1) == SOME_VALUE_1)
    > && (get_value(index2) == SOME_VALUE_1)
    > && (get_value(index3) == SOME_VALUE_1)
    > && (get_value(index4) == SOME_VALUE_1))
    > {
    > ....
    > }
    >
    > --
    > Ben.


    The other solution would be:

    /* we have a implementation for type bool */
    bool temp_val1 = (get_value(index1) == SOME_VALUE_1);
    bool temp_val2 = (get_value(index2) == SOME_VALUE_1);
    bool temp_val3 = (get_value(index3) == SOME_VALUE_1);
    bool temp_val4 = (get_value(index4) == SOME_VALUE_1);

    if (temp_val1 || temp_val2 || temp_val3 || temp_val4)
    {
    ....
    }

    But, like I already said else where in this thread, this is - in this
    case - tool satisfaction in our (not only my) opinion, which actually
    could have had unwanted side-effects in cases where the results for
    the different indexes would have been mutual exclusive. (Although in
    that case an if - elseif construction with explanatiory comment would
    have been a better readable / clearer and thus safer implementation-
    form, eventhough that would mean more lines of code)

    Kind regards,

    Harry
     
    Harry Ebbers, Nov 27, 2008
    #19
  20. Harry Ebbers

    Thad Smith Guest

    Ben Bacarisse wrote:
    > Thad Smith <> writes:
    >
    >> Harry Ebbers wrote:

    > <snip>
    >>> /* The lines marked with # are the lines where QA-C gives the
    >>> warning on */
    >>> if ((get_value(index1) == SOME_VALUE_1)
    >>> # || (get_value(index2) == SOME_VALUE_1)
    >>> # || (get_value(index3) == SOME_VALUE_1)
    >>> # || (get_value(index4) == SOME_VALUE_1))
    >>> {
    >>> ....
    >>> }
    >>> ....
    >>> }

    >> There are two ways I can think of that would probably avoid the warning:
    >>
    >> if ((get_value(index1) == SOME_VALUE_1)
    >> | (get_value(index2) == SOME_VALUE_1)
    >> | (get_value(index3) == SOME_VALUE_1)
    >> | (get_value(index4) == SOME_VALUE_1))
    >> {
    >> ....
    >> }
    >>
    >> That will work, but always calls get_value 4 times, rather than
    >> stopping with the first match of SOME_VALUE_1.
    >>
    >> The other is
    >>
    >> if (get_value(index1) != SOME_VALUE_1);
    >> else if (get_value(index2) != SOME_VALUE_1);
    >> else if (get_value(index3) != SOME_VALUE_1);
    >> else if (get_value(index4) != SOME_VALUE_1);
    >> else
    >> {
    >> ....
    >> }
    >>
    >> That is harder for me to read. I prefer your original code.

    >
    > Your second re-write is not the same thing at all! It is a form of
    >
    > if ((get_value(index1) == SOME_VALUE_1)
    > && (get_value(index2) == SOME_VALUE_1)
    > && (get_value(index3) == SOME_VALUE_1)
    > && (get_value(index4) == SOME_VALUE_1))
    > {
    > ....
    > }


    Hmmm, bad things happen to me when my code is hard to read. Thanks for
    catching that.

    Here's an even more perverse version (still not tested):

    do
    {
    if (get_value(index1) != SOME_VALUE_1)
    if (get_value(index2) != SOME_VALUE_1)
    if (get_value(index3) != SOME_VALUE_1)
    if (get_value(index4) != SOME_VALUE_1) break;
    /* Do something if any value equal */
    } while (0);

    --
    Thad
     
    Thad Smith, Nov 28, 2008
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Mr. SweatyFinger
    Replies:
    2
    Views:
    2,008
    Smokey Grindel
    Dec 2, 2006
  2. Why the compiler gives warning ?

    , Aug 9, 2005, in forum: C Programming
    Replies:
    13
    Views:
    537
  3. Borked Pseudo Mailed

    QA-C gives side-effects warning. Unknown why

    Borked Pseudo Mailed, Nov 29, 2008, in forum: C Programming
    Replies:
    2
    Views:
    319
    CBFalconer
    Dec 1, 2008
  4. Nomen Nescio

    QA-C gives side-effects warning. Unknown why

    Nomen Nescio, Dec 2, 2008, in forum: C Programming
    Replies:
    0
    Views:
    264
    Nomen Nescio
    Dec 2, 2008
  5. Tobias Weber

    Warning of missing side effects

    Tobias Weber, May 2, 2009, in forum: Python
    Replies:
    5
    Views:
    258
    Dave Angel
    May 2, 2009
Loading...

Share This Page