Take into account the types defined in stdint.h

[seb777]

Tokenizer::simplifyStdType must take into account the types defined in stdint.h (C99) / cstdin (C + +) and proceed to simplitfication.

See http://en.wikipedia.org/wiki/Stdint.h

Example of code:

int f() {
 uint8_t a;
 uint16_t b;
 uint32_t c;
 uint64_t d;
 int8_t e;
 int16_t f;
 int32_t g;
 int64_t h;
 return 0;
}

[hyd_danmar]

We shouldn't be too eager to add type info to the tokenizer. These types are not C90 as far as I know. I can't see much problems with handling these specific types but there are related types that I don't want hardcoded.

This could be solved with a improved support for taking type information from system headers. That is not very simple to fix though.

[hyd_danmar]

The conclusion..

It's ok that you add support for these specific types.

[seb777]

Patch to add C99 types.

[kimmov]

+        {
+            const char code[] = "void f(int x) {"
+                                "   uint32_t a;"
+                                "   return a+x;"
+                                "}";
+            ASSERT_EQUALS("void f ( int x ) { int a ; return a + x ; }", tok(code, true));
+        }
+
+        {
+            const char code[] = "void f(int x) {"
+                                "   uint64_t a;"
+                                "   return a+x;"
+                                "}";
+            ASSERT_EQUALS("void f ( int x ) { long long a ; return a + x ; }", tok(code, true));

I don't think you should make these assumptions if you want to keep Cppcheck portable.

[kimmov]

It's ok that you add support for these specific types.

It is unclear for me what this "support" means. If it is "support" by replacing the types defining sizes with other types then I disagree. There is a reason the header files exist. At least the attached patch seems to assume certain sizes for the types which is just wrong. Robert has been several times talking about e.g. DSP processors with different type sizes. Even for 64-bit architectures type sizes vary.

And of course we must make the distinction of:

• types in code checked
• types in platform Cppcheck is run

So one could think Cppcheck as cross-compiler. You should be able to check code for other platform. Like you should be able to check Windows code in Linux.

I've suggested somewhere earlier we should have simple compiler/platform specific configuration files where we would define e.g. type sizes. Another option is to find and parse (parts of) the system header files and determine them.

But please no hardcoding and assuming world is 32-bit (linux).

[hyd_danmar]

I am very sorry seb777 but I changed my mind. I don't want it to be solved through hard coding.

I've suggested somewhere earlier we should have simple compiler/platform specific configuration files where we would define e.g. type sizes. Another option is to find and parse (parts of) the system header files and determine them.

Yes this is how it should work.

[seb777]

At least the attached patch seems to assume certain sizes for the types which is just wrong.

Perhaps, the patch is based on linux man (http://linux.die.net/man/3/uint32_t) and the existing code, specifically this part:

-        if (Token::simpleMatch(tok, "__int8"))
-            tok->str("char");
-        else if (Token::simpleMatch(tok, "__int16"))
-            tok->str("short");
-        else if (Token::simpleMatch(tok, "__int32"))
-            tok->str("int");
-        else if (Token::simpleMatch(tok, "__int64"))
-        {
-            tok->str("long");
-            tok->isLong(true);
-        }

And of course we must make the distinction of:
* types in code checked
* types in platform Cppcheck is run

It is not clear. Yes, I agree to take into account the types in the code (original code). Of course!
Cppcheck must behave differently depending on the platform on which it runs ? In my opinion, no.
I think Cppcheck must ignore the exact size of types. It only needs to know size of:

char <= short <= int <= long (that would be abstract types used)

because it is true on all real compilers.

See:
* http://en.cppreference.com/w/cpp/language/types
* http://www.cplusplus.com/doc/tutorial/variables/

It is also true that:

int8_t <= int16_t <= int32_t <= int64_t

PS/ maybe the patch must be improved to simplify "long long" into "long".

[kimmov]

But please no hardcoding and assuming world is 32-bit (linux).

Perhaps, the patch is based on linux man

Sweet.

Cppcheck must behave differently depending on the platform on which it runs ? In my opinion, no.

Read again what I wrote, don't just argue for sake of argument.

I think Cppcheck must ignore the exact size of types

Then it cannot determine sizes of tables etc. E.g. the size in bytes of

• int8_t buffer[10] and
• int32_t buffer[10]

[edward-san]

I was thinking about the reverse thing: convert '(unsigned) int/char/long' types to (u)int*_t, which still could depend on the platform... but you know immediately which size they have.

[seb777]

Cppcheck must behave differently depending on the platform on which it runs ? In my opinion, no.

Read again what I wrote, don't just argue for sake of argument.

OK, but then I do not understand why you wrote that.

Another option is to find and parse (parts of) the system header files and determine them.

I do not understand either why hyd_danmar also agrees with this. This will be a mistake to do that like this, in my humble opinion.
I propose to talk about that in another discussion.

Then it cannot determine sizes of tables etc. E.g. the size in bytes of
* int8_t buffer[10] and
* int32_t buffer[10]

Yes, it is true.
Cppcheck will not be able to determine the exact size, but there is no need I think.
How can you be sure of the size calculation without knowing which platform the code is executed ?

Could you tell me in which case we need the exact size ? That's the most important question.

[kimmov]

I was thinking about the reverse thing: convert '(unsigned) int/char/long' types to (u)int*_t, which still could depend on the platform... but you know immediately which size they have.

Which has still exactly the same problem - you need to know from somewhere what is the size of the types.

I suggest reading this to begin with:
http://en.wikipedia.org/wiki/64-bit#64-bit_data_models

[kimmov]

Could you tell me in which case we need the exact size ?

When allocating memory, calculating pointers, accessing tables etc etc. Simple example below. Didn't try to compile or check, but you should get the idea.

void foo()
{
   int8_t buf[10] = {0};
   int64_t *ptr = (int64_t *)buf;
   ptr[4] = 15;
}

Does Cppcheck catch this? I hope it does. It should.

[kimmov]

I suggest reading this to begin with:
http://en.wikipedia.org/wiki/64-bit#64-bit_data_models

And in case it is not obvious - compare the size of long between Linux and Windows. And then tell me with straight face it doesn't matter. I've spent heck lot of hours with fighting getting some software even compile in 64-bit platform. And most of the problems are because of the wrong types and wrong assumptions about their sizes. So yes, I'm picky about this subject.

[kimmov]

And the fun begins when we use e.g. the long type whose size varies between common (32/64-bit) platforms. Consider this code in different platforms. Consider checking it with Cppcheck. What results you expect to get?

void foo()
{
   char* buf[16] = {0};
   long *ptr = (long *)buf;
   ptr[3] = 15;
}

I hope this helps you to understand why the size of types matter. And why the platform matters. I'm not saying this is easy problem (it is not). But we cannot forget this problem. After all we are doing a tool for finding this kind of errors from the code.

[kimmov]

There is ticket #2888 for type sizes configuration.

[hyd_danmar]

Kimmo.. sorry but I think that in your example it should be "char buf[16] = {0};". Without the "*". Otherwise I don't see the problem. Perhaps I miss something.

Cppcheck will not be able to determine the exact size, but there is no need I think.

You are right that there are many cases when we can determine there are bugs even though we don't know the exact size. I think it's extremely useful to catch such bugs.

It's just that there are many cases when we need to know the size to know if there is a bug or not.

[kimmov]

Perhaps I miss something.

In the later example? What happens in platform where long is 64 bits?

[kimmov]

And yes, in char* buf[16] the * is a typo. :(

[seb777]

OK, I see the problem.

1. We know that cppcheck is dependent on the machine on which it runs (see Tokenizer:: sizeOfType and the patch in the ticket #2939).

2. To my knowledge, Cppcheck does not really checks the correct memory size. The example given by kimmov produces no error / warning.

3. Cppcheck does not support (yet) the type C99 (original subject of this ticket).

I can make a dirty patch to support better the size of types. But, this will depend on the machine that turns cppcheck. This improves cppcheck quickly. Note that this feature seems to be expected (https://sourceforge.net/apps/phpbb/cppcheck/viewtopic.php?f=4&t=303&sid=93820fad4e7322ddd319ac955838374a)

Or we can also wait for the resolution of ticket #2939 and think about this ticket after.

How we want to proceed ?

[seb777]

Sorry, I talk about the ticket #2888

[kimmov]

I don't know how much #2888 will help. I mean if you want to map C99 types to base types you again have configuration problem - mapping varies by platform. Double configuration (first define base type sizes, then how C99 types map to them) doesn't sound nice.

OTOH, you can determine the size from the type name. Then you only need reverse mapping to base types? I.e. determine which base type is 32 bit unsigned integer.

[hyd_danmar]

I can make a dirty patch to support better the size of types.

yes.

A problem is: can we assume that uint32_t is a integer type? I know it's defined in C99 but it is not defined in C90. And it's not a builtin type in the compilers, so there is a small risk that the user has created his own special struct/union/etc with the name "uint32_t".

What is your opinion, can we assume that uint32_t is a integer type?

If we can assume that uint32_t is a integer/POD type then we can at least update Token::isStandardType. I believe that can be enough to fix the false negatives that was discussed in the forum.

[hyd_danmar]

In my humble opinion it should be safe to assume that uint32_t is a integer type. I'm interested to hear your opinions too.

My suggestion to update Token::isStandardType means that the checks won't be able to determine the size of the type but they will be able to determine that the type is a POD type. For many checks, the size doesn't matter and it won't make any difference if the size is known or not.

[seb777]

Update patch (infinite loop on intptr_t type).

[seb777]

Arggss.., fix correct size of uintptr_t

[kimmov]

+            else if (tokString.find("int32")!=std::string::npos ||
                   tokString.find("intptr")!=std::string::npos)
+            {
+                tok->str("int");

Pointer size is not the same size than int in all platforms. This exact assumption makes lots of Windows programs to broke when compiled 64-bit. And we have even a check for that in Cppcheck now...

Is this some board game where we reset to square one when day changes?

[seb777]

Kimmov, thank you for this information.
There are people who follow this patch on the forum. I'm just doing some fixes for them.

I understand the problems concerning the exact size of types.
Both, we know (and all who follow this ticket) that this patch is not correct as is in all platforms.
There is no magical solution. We'll have to wait what will be done in the ticket #2888.

[kimmov]

There are people who follow this patch on the forum. I'm just doing some fixes for them.

What is this forum I see referenced?

People following some forum is reason to add broken code nowadays? Getting things right isn't a priority anymore?

[hyd_danmar]

1.51 is released. removing milestone info from all open tickets