Magic constants and malloc() function

Sep 07 2009

Author: Andrey Karpov

Once again I would like to discuss the issue of using magic constants in code. We can eternally repeat that one should use sizeof() operator for correct calculation of the size of memory being allocated. But both this knowledge and correct writing of a new code will not help you detect an error already existing in the maze of the old code in large projects.

Let's consider a typical example of an error:

size_t nCount = 10;
int **poinerArray = (int **)malloc(nCount * 4);

The code is incorrect but in a 32-bit system it will work correctly. The error can occur when adapting the program to a different software/hardware environment. It has become very urgent and important to detect such a code because of mass migration of software on 64-bit systems. Changing of the sizes of some base types makes a code like this very dangerous. Viva64 analyzer included into PVS-Studio will show a warning about using magic constant "4″ on the code given above and an error will be detected when viewing the diagnostic warnings. But a code can be more complicated:

#define N_COUNT 100
#define POINTER_SIZE 4
#define NSIZE (N_COUNT * POINTER_SIZE)
int **pArray = (int **)malloc(NSIZE);

It is more difficult to diagnose an error in such a code written in C style with the use of #define. Although the code contains constant 4 defined by a macro, Viva64 analyzer is deliberately set so as to avoid showing warnings on such constructions. The analyzer ignores magic constants defined by macros (#define) due to two reasons. First, if a programmer defines constants through macros, he is likely to know what he is doing and a false response is very likely to occur. Second, if we react to constants which are dangerous from the viewpoint of a constant's 64-bit mode (4, 8, 32 etc) we will have too many false responses relating to using Windows API. Let's consider a harmless code as an example:

MessageBox("Are you sure ?",
           "Some question",
           MB_YESNO | MB_ICONQUESTION);

If we analyze the magic numbers hidden behind MB_YESNO and MB_ICONQUESTION macros there should be two warnings on using magic constants 4 and 32 on this line. Of course, it is too great a level of false responses. When analyzing malloc() function we can print all information about all dangerous magic constants without paying attention if it is a macro or not. But that is not enough anyway for the next case:

int **pArray = (int **)malloc(400);

If we go further and consider any number used in the expression for malloc() function unsafe it will cause false responses on a correct code:

int **pArray = (int **)malloc(400 * sizeof(int *));

On examining the situation, we have decided to introduce a new rule to verify applications whose result is transferred into malloc() function. At present, this rule reads as follows:

You should consider unsafe using numeric literals in the expression transferred into malloc() function. Exceptions:

1) The expression contains sizeof() operator

2) All the numeric literals divide by four with a remainder

Thanks to this rule we can warn about an error in the following code:

1) The first example:

void *p = malloc(nCount * 4);

2) The second example:

#define N_COUNT 100
#define POINTER_SIZE 4
#define NSIZE (N_COUNT * POINTER_SIZE)
int **pArray = (int **)malloc(NSIZE);

And also avoid showing a false warning on the code like:

1) The first example:

void *p = malloc(sizeof(double) * 4);

2) The second example:

#define N_COUNT 100
#define POINTER_SIZE sizeof(int *)
#define NSIZE (N_COUNT * POINTER_SIZE)
int **pArray = (int **)malloc(NSIZE);

This new diagnostic rule is most likely to appear in the next version of PVS-Studio 3.30. Let's now consider another situation also relating to malloc() function and incorrect suggestion about data alignment. It is not quite relative to magic constants but the problem is similar. Let's consider an example of code:

struct MyBigStruct {
  unsigned m_numberOfPointers;
  void *m_Pointers[1];
};
unsigned n = 10000;
void *ptr = malloc(sizeof(unsigned) +
                   n * sizeof(void *));

Although this code does not use magic numbers and the size of the types is defined by sizeof() the code is still incorrect. It does not take into account changing of the data alignment method different for 32-bit and 64-bit systems. The following code will be correct:

void *ptr = malloc(
  offsetof(MyBigStruct, m_Pointers) +
  n * sizeof(void *));

To warn the user about a possible error we are planning to introduce one more rule:

You should consider unsafe using more than one sizeof() operator in the expression transferred into malloc function. Perhaps, changing of alignment is not considered when calculating the structure's size.

In some cases this rule will cause false responses but such places must be checked thoroughly anyway.

The dangerous expressions with magic constants described above are topical not only for malloc() function but for a class of such functions as fread, fwrite etc. But these functions must be studied separately and we will perform their analysis later when diagnosis relating to malloc() function is completely worked out.

#Cpp #64bit