Thread: code checker crap

  1. #1
    Registered User
    Join Date
    Jan 2007
    Posts
    330

    code checker crap

    Hi,

    We use a code checker at work which complains about the following code:

    Code:
    short Rank;
    ....
    Rank = SHRT_MIN ;
    Code:
    // SHRT_MIN is defined as
    #define SHRT_MIN	(-32767-1)
    The complaint is
    "Values shall not be implicitly converted to a narrower type.
    (Category: CONVERSIONS, Level: 1)
    Value implicitly converted to narrower type 'short'"

    It's even a level 1 meaning that is a nasty bug according to the tool

    The tool says that because the expression (-32767-1) yields an int as type and that int is assigned to a short. So the fix to shut the tool up is

    Code:
    Rank = (short)SHRT_MIN;
    and management seems to think this improves code quality. What do you people think? The cast makes better quality code or not?

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Isn't SHRT_MIN already defined in <limits.h>?
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    I certainly don't think the cast improves the quality at all.

    Neither does gcc warn about any conversion when I do this:
    Code:
    #include <iostream>
    
    int main()
    {
      short a = -32767-1;
      std::cout << a << std::endl;
    }
    --
    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.

  4. #4
    Registered User
    Join Date
    Jan 2007
    Posts
    330
    Quote Originally Posted by laserlight View Post
    Isn't SHRT_MIN already defined in <limits.h>?
    yeah, its the one from the standard library. The code checker only checks code after it is preprocessed so it never sees SHRT_MIN but only (-32767-1)

  5. #5
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Chappell Hill, Texas
    Posts
    2,332
    I get a warning that SHRT_MIN is being redefined. Why not this (without a redefine):

    #define SHRTMIN (short) (-32767-1)

    Todd
    Mainframe assembler programmer by trade. C coder when I can.

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    yeah, its the one from the standard library. The code checker only checks code after it is preprocessed so it never sees SHRT_MIN but only (-32767-1)
    Oh, I see. Has it been pointed out to management that this is a problem with the code checker, since it purports to correct the standard library implementation? I too do not think that a cast is useful, but if it should be present, then the change should be made to <limits.h>.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by laserlight View Post
    Oh, I see. Has it been pointed out to management that this is a problem with the code checker, since it purports to correct the standard library implementation? I too do not think that a cast is useful, but if it should be present, then the change should be made to <limits.h>.
    But there is no conflict between -32768 (or -32767-1) and short (assuming of course that short is 16 bits, and not some other size).

    --
    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.

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    But there is no conflict between -32768 (or -32767-1) and short (assuming of course that short is 16 bits, and not some other size).
    Yes, so what is your point?
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by laserlight View Post
    Yes, so what is your point?
    Sorry, my point is that the code-checker is wrong, and not the original code. Possibly the difference between > and >=? (or < and <=).

    --
    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.

  10. #10
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Sorry, my point is that the code-checker is wrong, and not the original code.
    That is my point as well

    Possibly the difference between > and >=? (or < and <=).
    I do not think that the problem is so much of incorrect interpretation of the requirements for the standard minimum implementation limits, but that the code checker cannot recognise that the result is a constant that will fit into the range of the destination type, so there surely cannot be a potential bug lurking there, much less a nasty bug.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  11. #11
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by laserlight View Post
    That is my point as well


    I do not think that the problem is so much of incorrect interpretation of the requirements for the standard minimum implementation limits, but that the code checker cannot recognise that the result is a constant that will fit into the range of the destination type, so there surely cannot be a potential bug lurking there, much less a nasty bug.
    That is, of course another interpretation.

    Does it also complain if you assign it -32768, or -1?

    --
    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.

  12. #12
    Registered User
    Join Date
    Jan 2007
    Posts
    330
    Quote Originally Posted by matsp View Post
    That is, of course another interpretation.

    Does it also complain if you assign it -32768, or -1?

    --
    Mats
    yes it does because the checker sees any numerical constant as int type and assigning int to short without a cast is bad according to the checker. I agree that short x = int y; is not good code but with constants its a whole other story.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Extended ASCII Characters in an RTF Control
    By JustMax in forum C Programming
    Replies: 18
    Last Post: 04-03-2009, 08:20 PM
  2. Enforcing Machine Code Restrictions?
    By SMurf in forum Tech Board
    Replies: 21
    Last Post: 03-30-2009, 07:34 AM
  3. Obfuscated Code Contest: The Results
    By Stack Overflow in forum Contests Board
    Replies: 29
    Last Post: 02-18-2005, 05:39 PM
  4. Obfuscated Code Contest
    By Stack Overflow in forum Contests Board
    Replies: 51
    Last Post: 01-21-2005, 04:17 PM
  5. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM