Warnings when using % operator

Discussion in 'C Programming' started by Martin, Sep 15, 2004.

  1. Martin

    Martin Guest

    Below is some sample code to illustrate two warnings I get using a Hitachi
    compiler. When I use the GCC compiler I get a clean compile. I'd be grateful
    if someone could explain what the warnings actually mean as they are rather
    crytic, to me anyway.

    When I change the type of LOOKUP_MAX from size_t to int, I get a clean
    compile in the Hitachi compiler.

    #include <stdio.h>
    #define NELEMENTS(a) (sizeof(a) / sizeof(a[0]))

    /* macro for portable modulo operations */
    #if (-1 % 2 > 0)
    #define IMOD(i, j) ((i) % (j))
    #else
    #define IMOD(i, j) (((i) % (j)) < 0 ? ((i) % (j)) + j : ((i) % (j)))
    #endif

    char *lookup[] =
    {
    "ABC",
    "DEF",
    "GHI",
    "JKL"
    };

    const size_t LOOKUP_MAX = NELEMENTS(lookup);

    #include <stdio.h>

    int main( void )
    {
    int val;
    /* code where user decides whether to increment or decrement value
    removed */
    /* ... */

    /* User wants to increment value, keep in range [0, LOOKUP_MAX] */
    val = IMOD(val+1, LOOKUP_MAX);

    /* The above line of code generates two warnings:
    * Unsigned compare always true/false
    * Conditional expression always true/false.
    */
    return 0; /* success */
    }

    Martin
    http://martinobrien.co.uk/
    Martin, Sep 15, 2004
    #1
    1. Advertising

  2. Martin

    Ben Pfaff Guest

    "Martin" <martin.o_brien@[no-spam]which.net> writes:

    > #define IMOD(i, j) (((i) % (j)) < 0 ? ((i) % (j)) + j : ((i) % (j)))


    [...]

    > val = IMOD(val+1, LOOKUP_MAX);
    >
    > /* The above line of code generates two warnings:
    > * Unsigned compare always true/false
    > * Conditional expression always true/false.
    > */


    When `a' and `b' are unsigned, `a % b' is always nonnegative.
    Thus, ((i) % (j)) < 0 is always false in this case, hence the
    first warning. That subexpression is the first part of a ?:
    expression, hence the second warning.

    This kind of situation is why GCC doesn't generate these warnings
    by default. (You can enable the former with -W.)
    --
    Ben Pfaff
    email:
    web: http://benpfaff.org
    Ben Pfaff, Sep 15, 2004
    #2
    1. Advertising

  3. Martin

    Tim Rentsch Guest

    "Martin" <martin.o_brien@[no-spam]which.net> writes:

    > [...]
    >
    > /* macro for portable modulo operations */
    > #if (-1 % 2 > 0)
    > #define IMOD(i, j) ((i) % (j))
    > #else
    > #define IMOD(i, j) (((i) % (j)) < 0 ? ((i) % (j)) + j : ((i) % (j)))
    > #endif


    As other people have explained, the difficulty arose because the
    modulo operation was done unsigned because the second operand was
    unsigned.

    This brings up an interesting challenge - how can IMOD be written
    so that (1) it handles any combination of signed and unsigned
    operands, and (2) doesn't generate compiler warnings like the
    one mentioned in the earlier posting caused by comparing an
    unsigned value to be less than zero? The desired semantics
    are to return the smallest non-negative 'r' such that

    i == m * j + r

    with 'm' and 'r' being integers. For simplicity feel free to
    assume that j != 0 and the value of 'r' can be represented as
    a value of the type of 'i % j'.

    By the way, am I right in thinking that in a standard-compliant
    compilation the expression '-1 % 2 > 0' will always be zero (that
    is, false)?
    Tim Rentsch, Sep 16, 2004
    #3
  4. Tim Rentsch wrote:
    > This brings up an interesting challenge - how can IMOD be written
    > so that (1) it handles any combination of signed and unsigned
    > operands, and (2) doesn't generate compiler warnings like the
    > one mentioned in the earlier posting caused by comparing an
    > unsigned value to be less than zero? The desired semantics
    > are to return the smallest non-negative 'r' such that
    >
    > i == m * j + r
    >
    > with 'm' and 'r' being integers. For simplicity feel free to
    > assume that j != 0 and the value of 'r' can be represented as
    > a value of the type of 'i % j'.


    Neither assumption is needed: 0 <= r < |j|, and % takes the type of j.
    The answer seems simple to me - transform before, not afterwards:

    #define IMOD(i,j) ((i) <= 0 \
    ? ((j) <= 0 ? -j - ((-i) % (-j)) : j - ((-i) % (j))) \
    : ((j) <= 0 ? (i) % (-j) : (i) % (j)))

    Then, if i == m*j + r, then all the following also hold:
    i == (-m)*(-j) + r // i pos, j neg
    -i == (m-1)*(-j) - j - r // i neg, j neg, -j > r
    -i == (m-1)*j + j - r // i neg, j pos, j > r
    The remainder parts are all the smallest positive possible, since
    they're all positive and all < j.

    I'm avoiding the compiler warning for unsigned i and j by using "i <= 0"
    and "j <= 0" rather than "i < 0" and "j < 0" (it doesn't matter which
    branch we use for zero). Here's a complete sample that runs and produces
    correct answers with no warnings with gcc -Wall:

    #include <stdio.h>

    #define IMOD(i,j) ((i) <= 0 \
    ? ((j) <= 0 ? -j - ((-i) % (-j)) : j - ((-i) % (j))) \
    : ((j) <= 0 ? (i) % (-j) : (i) % (j)))

    int main() {
    printf("%d %d %d %d %d\n", IMOD(5u,3u), IMOD(5,3), IMOD(-5,3),
    IMOD(5,-3), IMOD(-5,-3));
    return 0;
    }

    > By the way, am I right in thinking that in a standard-compliant
    > compilation the expression '-1 % 2 > 0' will always be zero (that
    > is, false)?


    Nope. "If both operands are nonnegative then the remainder is
    nonnegative; if not, the sign of the remainder is
    implementation-defined." - ISO C++ Standard, 5.6

    However, there is an implicit constraint in this promise:
    "(a/b)*b + a%b is equal to a"
    But we know how multiplication and integer division behave; if you work
    it out, (a/b)*b must have the sign of a, and be no further from zero
    than a. Consequently, if a is negative, a%b must be nonpositive, in
    order to "get back" to a. Similarly, if a is positive, a%b must be
    nonnegative, regardless of the sign of b, to satisfy this constraint. I
    think pretty much all implementations overlooked this, though.
    --
    Derrick Coetzee
    I grant this newsgroup posting into the public domain. I disclaim all
    express or implied warranty and all liability. I am not a professional.
    Derrick Coetzee, Sep 16, 2004
    #4
  5. Derrick Coetzee wrote:
    > Nope. "If both operands are nonnegative then the remainder is
    > nonnegative; if not, the sign of the remainder is
    > implementation-defined." - ISO C++ Standard, 5.6
    > "(a/b)*b + a%b is equal to a"


    Oops, damnit. I thought I was in the C++ group. Very sorry. Although the
    C standard says very similar things in section 6.3.5, it also specifies
    that dividing can round in either direction (implementation-defined) if
    either operand is negative. This ruins my "implicit constraint"
    argument. However, the implementation's choice of behaviours for / and %
    are closely interlinked by the statement above.
    --
    Derrick Coetzee
    I grant this newsgroup posting into the public domain. I disclaim all
    express or implied warranty and all liability. I am not a professional.
    Derrick Coetzee, Sep 16, 2004
    #5
  6. Martin

    CBFalconer Guest

    Derrick Coetzee wrote:
    > Derrick Coetzee wrote:
    >
    >> Nope. "If both operands are nonnegative then the remainder is
    >> nonnegative; if not, the sign of the remainder is
    >> implementation-defined." - ISO C++ Standard, 5.6
    >> "(a/b)*b + a%b is equal to a"

    >
    > Oops, damnit. I thought I was in the C++ group. Very sorry.
    > Although the C standard says very similar things in section 6.3.5,
    > it also specifies that dividing can round in either direction
    > (implementation-defined) if either operand is negative. This ruins
    > my "implicit constraint" argument. However, the implementation's
    > choice of behaviours for / and % are closely interlinked by the
    > statement above.


    I believe the standard for % changed between C90 and C99. So it
    might be wide to make the macro depend on the standard version.
    This won't help with non-standard systems, such as many embedded
    compilers.

    My preference is that the modulo operation adjust the signs so
    that the denominator is positive, and then return a value with the
    sign of the numerator. I don't think the standards agree with me.

    --
    "This is a wonderful answer. It's off-topic, it's incorrect,
    and it doesn't answer the question." -- Richard Heathfield

    "I support the Red Sox and any team that beats the Yankees"
    CBFalconer, Sep 16, 2004
    #6
  7. CBFalconer wrote:
    > I believe the standard for % changed between C90 and C99. So it
    > might be wide to make the macro depend on the standard version.
    > This won't help with non-standard systems, such as many embedded
    > compilers.


    It's changed significantly actually, but only in the last clause:

    C90, 6.3.5: "When integers are divided and the division is inexact, if
    both operands are positive the result of the / operator is the largest
    integer less than the algebraic quotient and the result of the %
    operator is positive. If either operand is negative, whether the result
    of the / operator is the largest integer less than or equal to the
    algebraic quotient or the smallest integer greater than or equal to the
    algebraic quotient is implementation-defined, as is the sign of the
    result of the % operator. If the quotient a/b is representable, the
    expression (a/b)*b + a%b."

    C99, 6.5.5: "When integers are divided, the result of the / operator is
    the algebraic quotient with any fractional part discarded. [Footnote:
    This is often called "truncation towards zero."] If the quotient a/b is
    representable, the expression (a/b)*b + a%b shall equal a."

    Although much shorter, this is a significantly stronger restriction on
    both / and %. They've mandated that division is now truncation, even for
    negative arguments, for example:

    (-5)/(-3) = 1 or 2 in C90, but 1 in C99
    (-5)/3 = -1 or -2 in C90, but -1 in C99

    Among other things, this means that this identity now holds:

    a/b == (-a)/(-b), for all values a,b of integral types where -a and -b

    are representable

    As a consequence, my previous argument applies in C99, and ensures that
    a%b has the sign of a (considering zero to have both signs). Although
    the sign of % appears to now be unspecified, in reality it's fully
    specified, but in a very subtle way.
    --
    Derrick Coetzee
    I grant this newsgroup posting into the public domain. I disclaim all
    express or implied warranty and all liability. I am not a professional.
    Derrick Coetzee, Sep 16, 2004
    #7
  8. Martin

    Tim Rentsch Guest

    Derrick Coetzee <> writes:

    > Tim Rentsch wrote:
    > > This brings up an interesting challenge - how can IMOD be written
    > > so that (1) it handles any combination of signed and unsigned
    > > operands, and (2) doesn't generate compiler warnings like the
    > > one mentioned in the earlier posting caused by comparing an
    > > unsigned value to be less than zero? The desired semantics
    > > are to return the smallest non-negative 'r' such that
    > >
    > > i == m * j + r
    > >
    > > with 'm' and 'r' being integers. For simplicity feel free to
    > > assume that j != 0 and the value of 'r' can be represented as
    > > a value of the type of 'i % j'.

    >
    > Neither assumption is needed: 0 <= r < |j|, and % takes the type of j.
    > The answer seems simple to me - transform before, not afterwards:


    Certainly there can't be an 'r' with 0 <= r < |j| when j is zero.
    A literal reading of the constraint in my earlier message would mean
    that j == 0 would mean r == i if i >= 0, and no possible value for
    r if i < 0. But let's ignore the j == 0 case for now.


    > #define IMOD(i,j) ((i) <= 0 \
    > ? ((j) <= 0 ? -j - ((-i) % (-j)) : j - ((-i) % (j))) \
    > : ((j) <= 0 ? (i) % (-j) : (i) % (j)))


    I like the approach. Unfortunately it doesn't quite work if i has a
    value that is the most negative value for the type (on most machines
    with 2's complement representation anyway). (See I wasn't asking a
    question that's completely trivial. :) If j's value is the most
    negative value for its (signed) type, the tests I did show the right
    value, but it seems like that depends on how signed overflow behaves,
    which can't be relied on standard-wise (at least, I believe it can't).


    > > By the way, am I right in thinking that in a standard-compliant
    > > compilation the expression '-1 % 2 > 0' will always be zero (that
    > > is, false)?

    >
    > [...]
    >
    > However, there is an implicit constraint in this promise:
    > "(a/b)*b + a%b is equal to a"
    > But we know how multiplication and integer division behave; if you work
    > it out, (a/b)*b must have the sign of a, and be no further from zero
    > than a. Consequently, if a is negative, a%b must be nonpositive, in
    > order to "get back" to a. Similarly, if a is positive, a%b must be
    > nonnegative, regardless of the sign of b, to satisfy this constraint.


    Right. I was thinking that the combination of the constraint relating
    the '/' and '%' operators plus knowing that divide rounds towards zero
    together would imply what the sign of -1 % 2 would be. I suppose
    the implementation might be non-compliant, however.
    Tim Rentsch, Sep 17, 2004
    #8
  9. Martin

    Tim Rentsch Guest

    Derrick Coetzee <> writes:

    > Derrick Coetzee wrote:
    > > Nope. "If both operands are nonnegative then the remainder is
    > > nonnegative; if not, the sign of the remainder is
    > > implementation-defined." - ISO C++ Standard, 5.6
    > > "(a/b)*b + a%b is equal to a"

    >
    > Oops, damnit. I thought I was in the C++ group. Very sorry. Although the
    > C standard says very similar things in section 6.3.5, it also specifies
    > that dividing can round in either direction (implementation-defined) if
    > either operand is negative.


    Oh, I thought C99 said (in effect) that division would round toward
    zero. Maybe I'm misremembering.
    Tim Rentsch, Sep 17, 2004
    #9
  10. Martin

    Ben Pfaff Guest

    Tim Rentsch <> writes:

    > Oh, I thought C99 said (in effect) that division would round toward
    > zero. Maybe I'm misremembering.


    C99 6.5.5 says this:

    When integers are divided, the result of the / operator is
    the algebraic quotient with any fractional part
    discarded.87) If the quotient a/b is representable, the
    expression (a/b)*b + a%b shall equal a.

    87) This is often called ``truncation toward zero''.
    --
    Here's a tip: null pointers don't have to be *dull* pointers!
    Ben Pfaff, Sep 17, 2004
    #10
  11. Martin

    Martin Guest

    Thank you Ben, Tim, Derrick, and CBFalconer for your replies.

    It is a real nuisance that I get that warning, I have now had to change my
    initial strategy of creating variables using this method:

    const size_t LOOKUP_MAX = NELEMENTS(lookup);

    to

    const int LOOKUP_MAX = NELEMENTS(lookup);

    Obviously I prefer the former as NELEMENTS (the macro I use and shown in my
    initial post), uses the sizeof operator which returns size_t and there is
    the potential for the wrong value to be placed in LOOKUP_MAX if it's greater
    than INT_MAX. This is unlikely to happen in the code I'm writing, but alert!
    That's famous last words isn't it? And is how bugs are conceived.

    I know that Ben said GCC doesn't generate those warning by default, is it
    just those specific type of warnings? I'd have to check in the Hitachi
    compiler whether that level of fine-tuning is available.

    Martin
    Martin, Sep 28, 2004
    #11
  12. Martin

    Eric Sosman Guest

    Martin wrote:
    > Thank you Ben, Tim, Derrick, and CBFalconer for your replies.
    >
    > It is a real nuisance that I get that warning, I have now had to change my
    > initial strategy of creating variables using this method:
    >
    > const size_t LOOKUP_MAX = NELEMENTS(lookup);
    >
    > to
    >
    > const int LOOKUP_MAX = NELEMENTS(lookup);
    >
    > Obviously I prefer the former as NELEMENTS (the macro I use and shown in my
    > initial post), uses the sizeof operator which returns size_t and there is
    > the potential for the wrong value to be placed in LOOKUP_MAX if it's greater
    > than INT_MAX. This is unlikely to happen in the code I'm writing, but alert!
    > That's famous last words isn't it? And is how bugs are conceived.


    Opening the gates to potential bugs in order to
    shut out warnings seems a poor trade-off. Why not

    /* macro for portable modulo operations */
    #if (-1 % 2 > 0)
    #define IMOD(i, j) ((i) % (j))
    #else
    #define IMOD(i, j) (((i) % (j) + (j)) % (j))
    #endif

    By the way, neither this nor the original handles
    the case of j<0 -- but if j is a size_t, that's not
    too much of a problem ...

    --
    Eric Sosman, Sep 28, 2004
    #12
  13. Martin

    Martin Guest

    "Eric Sosman" wrote:
    > Opening the gates to potential bugs in order to
    > shut out warnings seems a poor trade-off. Why not
    >
    > /* macro for portable modulo operations */
    > #if (-1 % 2 > 0)
    > #define IMOD(i, j) ((i) % (j))
    > #else
    > #define IMOD(i, j) (((i) % (j) + (j)) % (j))
    > #endif
    >
    > By the way, neither this nor the original handles
    > the case of j<0 -- but if j is a size_t, that's not
    > too much of a problem ...


    Have you read my original post, or this thread, Eric?

    Martin
    Martin, Sep 28, 2004
    #13
  14. Martin

    Eric Sosman Guest

    Martin wrote:
    > "Eric Sosman" wrote:
    >
    >> Opening the gates to potential bugs in order to
    >>shut out warnings seems a poor trade-off. Why not
    >>
    >>/* macro for portable modulo operations */
    >>#if (-1 % 2 > 0)
    >>#define IMOD(i, j) ((i) % (j))
    >>#else
    >>#define IMOD(i, j) (((i) % (j) + (j)) % (j))
    >>#endif
    >>
    >>By the way, neither this nor the original handles
    >>the case of j<0 -- but if j is a size_t, that's not
    >>too much of a problem ...

    >
    >
    > Have you read my original post, or this thread, Eric?


    Yes, but this latest question baffles me. What
    are you getting at?

    --
    Eric Sosman, Sep 28, 2004
    #14
  15. Martin

    Tim Rentsch Guest

    "Martin" <martin.o_brien@[no-spam]which.net> writes:

    > Thank you Ben, Tim, Derrick, and CBFalconer for your replies.
    >
    > It is a real nuisance that I get that warning, I have now had to change my
    > initial strategy of creating variables using this method:
    >
    > const size_t LOOKUP_MAX = NELEMENTS(lookup);
    >
    > to
    >
    > const int LOOKUP_MAX = NELEMENTS(lookup);
    >
    > Obviously I prefer the former as NELEMENTS (the macro I use and shown in my
    > initial post), uses the sizeof operator which returns size_t and there is
    > the potential for the wrong value to be placed in LOOKUP_MAX if it's greater
    > than INT_MAX. This is unlikely to happen in the code I'm writing, but alert!
    > That's famous last words isn't it? And is how bugs are conceived.


    If I understand the problem you're dealing with, it's that the first
    form (where LOOKUP_MAX is unsigned) causes problems because some
    signed variable (probably 'int i') is compared to it. So if
    there is code like

    int i;

    for( i = 0; i < LOOKUP_MAX; i++ ){
    ... something with lookup ...
    }

    The signed/unsigned comparison in the 'for' control clause gets flagged
    with a warning. Am I on track?

    Assuming that's the kind of problem you're facing, another approach is
    to use unsigned types for variables used for indexing. I've taken to
    using the type name 'Index' for this:

    Index i;

    for( i = 0; i < LOOKUP_MAX; i++ ){
    ... something with lookup ...
    }

    where 'Index' has been typedef'ed to 'unsigned int' or 'unsigned long'
    (or conceivably some other unsigned integer type). Of course, if
    this is done, other warnings will come up when 'i' is compared to
    signed quantities. Sometimes those quantities can also be changed
    to unsigned, but when they can't, it's nice to be able to write
    warning free, and safe, comparisons:

    #define NONNEGATIVE(x) (assert((x)>=0),(x))

    Index i;
    int dollars;

    if( i > (Index) NONNEGATIVE(dollars) ){
    ...
    }

    I have to admit, getting around the signed/unsigned comparison
    warnings is kind of a pain in the a**, and it takes some getting used
    to. Now that I've been doing it for a while though, it does seem like
    the benefits outweigh the costs. And, I'm happy to report, the effort
    required to avoid the warnings has gone down monotonically as I've
    gone up the experience curve.

    By the way, note that the way NONNEGATIVE is written, many compilers
    will give a warning message if the argument is already an unsigned
    type (as might happen if the type of 'dollars' in the code above
    changed, for example). I consider that warning a plus rather than a
    minus - it means NONNEGATIVE won't be used unnecessarily. I'm
    confident that there are other ways to write the assertion so that no
    warning will be given for already unsigned arguments, if someone were
    to desire that.
    Tim Rentsch, Sep 29, 2004
    #15
  16. Martin

    Martin Guest

    Tim wrote:
    > ... The signed/unsigned comparison in the
    > 'for' control clause gets flagged with a
    > warning. Am I on track?


    Sort of. In fact, Ben explained the problem in his response to me:

    "When `a' and `b' are unsigned, `a % b'
    is always nonnegative. Thus, ((i) % (j)) < 0
    is always false in this case, hence the
    first warning. That subexpression is the
    first part of a ?: expression, hence the
    second warning." -- Ben, 15 Sep 2004

    So the problem arises when the IMOD macro is used. Regarding your
    suggestion, I note that Plauger says:

    "You should make a point of using type
    size_t *anywhere* your program
    performs array subscripting or address
    arithmetic."
    -- P.J. Plauger, The Standard C Library

    Thanks for your post.

    Martin
    http://martinobrien.co.uk/
    Martin, Sep 29, 2004
    #16
  17. Martin

    Martin Guest

    "Eric Sosman" wrote:
    > Yes, but this latest question baffles me. What
    > are you getting at?


    Eric, I was puzzled by the sudden break in sentence, you wrote "why not" and
    then quoted my IMOD definition.

    Martin
    http://martinobrien.co.uk/
    Martin, Sep 29, 2004
    #17
  18. Martin

    Eric Sosman Guest

    Martin wrote:
    > "Eric Sosman" wrote:
    >
    >> Yes, but this latest question baffles me. What
    >>are you getting at?

    >
    >
    > Eric, I was puzzled by the sudden break in sentence, you wrote "why not" and
    > then quoted my IMOD definition.


    Ah, now I see. If you'll re-read my post you'll find
    that one of the six lines is not a quote, but differs from
    your original version. The changed line is my contribution
    (FWIW); the others were just for context. Sorry if they
    misled you.

    --
    Eric Sosman, Sep 30, 2004
    #18
    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. Manish_Ganvir
    Replies:
    13
    Views:
    1,559
    Keith Thompson
    Feb 14, 2005
  2. Stephan Kämper

    Warnings when using RedCloth and Rake

    Stephan Kämper, Jun 22, 2004, in forum: Ruby
    Replies:
    8
    Views:
    103
    Eric Hodel
    Jun 23, 2004
  3. Suresh Govindachar

    Using strict and warnings in embedded perl

    Suresh Govindachar, Jul 14, 2004, in forum: Perl Misc
    Replies:
    0
    Views:
    104
    Suresh Govindachar
    Jul 14, 2004
  4. Ted Sung
    Replies:
    1
    Views:
    299
    Sherm Pendley
    Aug 30, 2004
  5. W Karas

    GCC warnings for comma operator

    W Karas, Apr 11, 2013, in forum: C Programming
    Replies:
    16
    Views:
    458
Loading...

Share This Page