Dangerous magic number N used

Question

PVS-Studio, the static code analyzer, for the following bit of code

size_t const n = 4;
int a[n] = {};

reports:

V112 Dangerous magic number 4 used: ...t const n = 4;. test.cpp 3

Although PVS-Studio is used with Visual Studio 2017 project and reports the same warning for both, 32 and 64 bit, those build configurations are not taken into account by the analyzer, AFAIU.

I would have expected the context to be analysed better and treat the code above as equivalent to this

int a[4] = {};

for which PVS-Studio does not issue any diagnostics.

In the case above is this dangerous magic number N used, a false positive?

What are the reasons the two code samples above are not analyzed as equivalent?

Vidal

If anything I would assume the second example's use of magic numbers is less safe than the first one, making it more worthy of a warning. It seems that the warning is related to n = 4 and not with the use of n as an array size. – Orangeade

I'm personally not very impressed with the quality of PVS Studios analysis, and this result is in line with what I expect from it - low quality warning - not all that useful. – Tice

It is all explained in the link you got from PVS-Studio and posted. Even why int a[4] doesn't get warning. – Ovipositor

@MarekVitek Unless the page has been updated over the last 48h, it does neither explain specifically why the former does and why the latter doesn't issue the warning. Mind you, the last paragraph is not satisfactory explanation. – Vidal

@FrançoisAndrieux Yes, indeed. – Vidal

@JesperJuhl I don't want to turn this QA into review of PVS-Studio. FWIW, I'm not entirely satisfied with VS2017 analyzer myself, for instance. – Vidal

My understanding is that the analyzer is complaining about the number 4. As an experiment, try "#define ARRAY_CAPACITY (4U), instead of const size_t n = 4;`. You may need to configure the analyzer for C++. – Calci

@ThomasMatthews Preprocessor macro is 1:1 equivalent to the second case a[4]. Complains about number 4 is clear, but how on earth one is supposed to use constant/literal without putting there the 4 ;-) – Vidal

@Vidal Number 4 is mentioned in the table as one of the numbers that might be causing troubles when porting to 64 bit and require special attention. int a[4] is mentioned as special case that gets exempted from the check. Use 5 as your constant and there will be no warning. – Ovipositor

Or better yet suppress this one instance add //-V112 after line where warning is issued. – Ovipositor

The static analyzer may have rules requiring that numbers other than 0 or 1 be a macro; regardless of the equivalency to the second case. – Calci

@MarekVitek Is that seriously and true? I have read the table but as reader, I took "Let's list magic numbers" as "let's list all magic numbers we can see in the examples above", but not as "let's list PVS-specific magic numbers". I suspect, most readers would understand it that way, no? I clearly didn't pay enough attention to the table content besides the 4 or 0xffffffff :-) FYI, n=4 comes from nanodbc library tests, what a coincidence! – Vidal

Is that seriously and true? Yes. The magic numbers (for detect 64-bit issues) is 4, 32, 0x7FFFFFFF, 0x80000000 and 0xFFFFFFFF. Details: viva64.com/en/l/0009 – Plumbaginaceous

@Plumbaginaceous OK. It would be helpful if this article is linked from the docs though. MarekVitek thanks for digging this up! – Vidal

@Vidal Yes it is seriously true. These numbers have special meaning in lot of code hence the special attention to it. The reason is because in many cases their meaning changes with transition to 64 bit system. – Ovipositor

@MarekVitek I stand corrected. I haven't paid enough attention reading the docs page, apparently. Thanks – Vidal

@Vidal Well you are right in one point. The doc page could use bit of rework. Move the table to the top would be first step. And little bit change of wording might help too. But I am not an user of that tool. – Ovipositor

Answer 1

This

size_t const n = 4;
int a[n] = {};

is false positive.

64-bit diagnostics are very noisy and there is nothing you can do about it. Yes, the analyzer produces many false positives such as magic numbers like 4, 0xFFFFFFFF, etc. In the analyzer a lot of exceptions has already been made when it doesn’t complain (for example: int a[4] = {};). However, there are still so many options of using constants that it’s impossible to foresee all of them.

When porting code to 64-bit system it makes sense to look through all the magic numbers, to make sure that the programmer, for example, does not expect that the pointer size is 4 bytes somewhere. Then it makes sense to switch off V112 diagnostic so that it does not bother you.

Plumbaginaceous

Where did you come up with 64-bit? I didn't see it in the example lines. This could be on a 32-bit or 16-bit system. – Calci

I have updated my question to clarify I use PVS with VS2017 – Vidal

> "Where did you come up with 64-bit?" This diagnostic was created for detect 64-bit issues. viva64.com/en/l/0009 – Plumbaginaceous

@Plumbaginaceous Can you relate to the comments by MarekVitek above? If that is correct, may I suggest to re-phrase the docs as "Let's list specific magic numbers that PVS-Studio is especially concerned about..." or something alike – Vidal

I did not understand the question. The magic numbers (for detect 64-bit issues) is 4, 32, 0x7FFFFFFF, 0x80000000 and 0xFFFFFFFF. – Plumbaginaceous

Answer 2

Reading the link you posted, I concluded it is a false positive in your case.

The tool is assuming you are going to use n in a malloc (or equivalent allocation procedure) statement to be equivalent to the size of int (or any 4 bytes variable). So the recommendation is to use sizeof(desired type).

If you were using n inside a malloc statement, it would make sense - since int (or any other type) could vary for different architectures (if not now, in the future). But apparently this is not your case.

Rosenzweig

How do you get the assumption of using malloc? The OP is declaring an automatic variable, not using malloc or new. The constant identifier could be used in assignment or arithmetic expressions. – Calci

As I said, my conclusion was based on what I have read on the link posted on the question: viva64.com/en/w/V112/print - It uses malloc as example to explain it. – Rosenzweig

@ThomasMatthews Because PVS-Studio apparently doesn't go on deep analysis where the variable is used. It sees const being assigned 4 and issues warning. But maybe I am wrong. – Ovipositor

Answer 3

Number 4 is considered as one of the potentially dangerous numbers when porting from 32 bit to 64 bit hence warning on const being assigned 4. Other numbers are listed in the table behind the link you posted. With examples how it can be dangerous.

You can suppress individual warning by adding //-V112 at the end of line you are sure is 100% OK.

size_t const n = 4; //-V112

This will suppreess the warning and you can focus on your work again.

As for int a[4] = {}; PVS-Studio considers it a special case for which it doesn't issue warning. Why it doesn't take it into account in the first case I don't know. But it looks like hardcoded exception for really specific case.

If you are not building 64 bit builds, then I assume it is safe to disable the warning alltogether for now. But be warned - comes from sight comes from mind.

Ovipositor