Thread: Fixing signed/unsigned warnings?

  1. #1
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545

    Fixing signed/unsigned warnings?

    I'm trying to compile some very bad code that gives me dozens of warnings like:
    Code:
    warning C4018: '>=' : signed/unsigned mismatch
    for code such as:
    Code:
    if ( wcslen(comma) >= bufLen )
    and I'm wondering what the best way to get rid of these warnings?
    I can't make bufLen unsigned because it would break the API that's already used by many different products.

    I can see 3 choices:
    - Use a #pragma to disable the warning, but that's just sloppy and compiler specific.
    - Cast the unsigned int to an int.
    - Cast the int to an unsigned int.

    Do I just flop a coin about which side to cast, or is one choice better than the other?

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    My guess is, if these are both supposed to represent lengths, then you should cast to unsigned int. Do you know how bufLen gets a negative value? Is it set to a negative sentinel, like -1, that you should watch for, or is it just so large that it's stored negative?

  3. #3
    Registered User
    Join Date
    Sep 2006
    Posts
    230
    Well, it depends on the possible values of the two variables. Example: if the unsigned is actually a size_t and the signed is a counter, then the obvious choice is to cast unsigned to the counter (int) since there's almost no risk (if any) of losing any data.

    EDIT: After looking at it again (and seeing tabstop's reply) I realized that they are both lengths so, I guess you should just cast unsigned to the int.

  4. #4
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Thanks,
    I guess if a negative buf length is passed in that's a whole other bug, so I just casted it to unsigned int.

    Now, what about variables that aren't lengths, and are completely undocumented (just like most of this code)?

    BTW, what's the default conversion that the compiler makes if you just ignore the warning? Does it convert them both to int, or both to unsigned ints?

  5. #5
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    I think the integer promotions are the same in C++ as in C. The unsigned versions are always of higher rank than the signed versions, so I would guess things would promote to unsigned. (I'm not in the same place as my standard, and it's a C standard, not C++ anyway.)

  6. #6
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    Why is bufLength signed in the first place?
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  7. #7
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by CornedBee View Post
    Why is bufLength signed in the first place?
    Because this code sucks. It also has port numbers as DWORDs instead of unsigned shorts, and it checks if ( some_unsigned_number < 0 )...

  8. #8
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    Quote Originally Posted by tabstop View Post
    I think the integer promotions are the same in C++ as in C. The unsigned versions are always of higher rank than the signed versions, so I would guess things would promote to unsigned. (I'm not in the same place as my standard, and it's a C standard, not C++ anyway.)
    Yes, that's how it is in C++ as well as in C. They wouldn't have changed such a basic rule.

    If the code always uses signed variables like this, you could use
    Code:
    #define wcslen(str) ((signed long)wcslen(str))
    or something. An ugly hack in a sea of them, no doubt. I'd probably just ignore the warnings if I was just trying to compile the code.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  9. #9
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by dwks View Post
    Yes, that's how it is in C++ as well as in C. They wouldn't have changed such a basic rule.

    If the code always uses signed variables like this, you could use
    Code:
    #define wcslen(str) ((signed long)wcslen(str))
    or something. An ugly hack in a sea of them, no doubt. I'd probably just ignore the warnings if I was just trying to compile the code.
    I hate ignoring warnings, because eventually you'll have so many of them that you'll miss the important ones. I already found a few serious bugs by fixing the warnings; but I don't want to start pulling at too many loose threads since the whole thing might fall apart.

  10. #10
    Registered User
    Join Date
    Oct 2001
    Posts
    2,129
    Why don't you make some tests to run after each build or something?

    Unit testing or something. I don't know how to do it, just that it might be useful.

  11. #11
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by dwks View Post
    Yes, that's how it is in C++ as well as in C. They wouldn't have changed such a basic rule.
    Yes, but I just had weird visions of
    Code:
    bool operator < (const int& left, const unsigned int& right)
    running through my mind. I need to get out more.

  12. #12
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Perhaps juse make bufLen unsigned anyway, and use a cast in the calls to the API.
    That makes it obvious where the incorrect type is being used - the API itself.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  13. #13
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by robwhit View Post
    Why don't you make some tests to run after each build or something?

    Unit testing or something. I don't know how to do it, just that it might be useful.
    It's not my code. I'm just fixing a bug in it since the person that normally takes care of it is busy with another project. Trust me, if it was my code it would be running like a well oiled machine.

  14. #14
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by iMalc View Post
    Perhaps juse make bufLen unsigned anyway, and use a cast in the calls to the API.
    That makes it obvious where the incorrect type is being used - the API itself.
    I'm not very familiar with the code and I certainly don't know what other code calls this function, so I'm just leaving it as is. I'm just casting away the warning and adding a big ugly comment about the warning I was getting...

  15. #15
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    I would also add an assert(buflen >= 0) (or if-statement with a printout, or similar) just above the line with the cast [if there are several casts on the same (unchanged) variable, you obviously only need one assert]. That way, _IF_ the bufLen is ever negative, you have a chance to catch it.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Advice on removing valid (but minor) compiler warnings
    By PaulBlay in forum C Programming
    Replies: 12
    Last Post: 04-20-2009, 12:16 PM
  2. Warnings when using vector of vector
    By Boksha in forum C++ Programming
    Replies: 5
    Last Post: 03-29-2008, 01:54 PM
  3. Replies: 9
    Last Post: 03-14-2008, 09:55 AM
  4. Screwy Linker Error - VC2005
    By Tonto in forum C++ Programming
    Replies: 5
    Last Post: 06-19-2007, 02:39 PM
  5. Warnings, warnings, warnings?
    By spentdome in forum C Programming
    Replies: 25
    Last Post: 05-27-2002, 06:49 PM