2
votes
#define MAX 265

std::cout << 0 * MAX << std::endl; //to my surprise, the output is 9 rather than 0

What's the problem with this C++ macro multiplication?

EDIT:

The following is the complete version.

#include <stdio.h>
#include <string.h>
#include <iostream>

#define NAME_BYTES 256
#define VERSION_BYTES 256
#define SIZE_BYTES 32
#define USED_LOCK_COUNT_BYTES 32
#define LOCK_NAME_BYTES 256
#define LOCK_TYPE_BYTES 1
#define PID_BYTES 4
#define TID_BYTES 4
#define LOCK_BYTES LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES 
#define HEADER_BYTES NAME_BYTES + VERSION_BYTES + SIZE_BYTES + USED_LOCK_COUNT_BYTES

int main() {
  std::cout << "LOCK_BYTES: " << LOCK_BYTES << std::endl;
  std::cout << "HEADER_BYTES: " << HEADER_BYTES << std::endl;
  std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_BYTES << std::endl;
}

Here's the result I just got and the compiler info.

yifeng@yifeng-Precision-WorkStation-T3400:~/Shared-Memory-Solution/examples/IMPL$ g++ -v Using built-in specs. COLLECT_GCC=g++ COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6.1/lto-wrapper Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 4.6.1-9ubuntu3' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++,go --prefix=/usr --program-suffix=-4.6 --enable-shared --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.6.1 (Ubuntu/Linaro 4.6.1-9ubuntu3)

yifeng@yifeng-Precision-WorkStation-T3400:~/Shared-Memory-Solution/examples/IMPL$ ./a.out LOCK_BYTES: 265 HEADER_BYTES: 576 LOCK_BYTES * 0: 9

EDIT: Thank you so much guys!! I've been most happy that I decided to post this, although I am getting so many downvotes. What a lesson to learn about MACRO!

6

6 Answers

11
votes

You should always put parentheses around macro definitions:

#define LOCK_BYTES (LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES)

Otherwise, the code expands to:

cout << 0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

which outputs the value of LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES.

Better still, don't use macros unless you really have to. These are better represented as constant variables.

9
votes
std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_BYTES << std::endl;

Expands to

std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES << std::endl;

Which in turn expands to

std::cout << "LOCK_BYTES * 0: " << 0 * 256 + 1 + 4 + 4 << std::endl;

And with added parens for the precedence rules:

std::cout << "LOCK_BYTES * 0: " << ((((0 * 256) + 1) + 4) + 4) << std::endl;

Which evaluates to

std::cout << "LOCK_BYTES * 0: " << 9 << std::endl;

Change your code to

std::cout << "LOCK_BYTES * 0: " << 0 * (LOCK_BYTES) << std::endl;

Or even better, use const unsigned int values:

const unsigned int NAME_BYTES = 256;
const unsigned int VERSION_BYTES = 256;
const unsigned int SIZE_BYTES = 32;
const unsigned int USED_LOCK_COUNT_BYTES = 32;
const unsigned int LOCK_NAME_BYTES = 256;
const unsigned int LOCK_TYPE_BYTES = 1;
const unsigned int PID_BYTES = 4;
const unsigned int TID_BYTES = 256;
const unsigned int LOCK_BYTES = LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES;
const unsigned int HEADER_BYTES = NAME_BYTES + VERSION_BYTES + SIZE_BYTES + USED_LOCK_COUNT_BYTES;

Huzzah! And suddenly, you don't have weird problems anymore.

6
votes

I think you've miscalculated a bit. macros do not act like variables, their values are inserted in their place before compilation by the preprocessor. So therefore, what the compiler will see is:

 0* LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES 

which is:

 0 * 256 + 1 + 4 + 4

which according to the Order of Operations, on which C++'s operator precedence is based, multiplication happens first, so it will equal 9 not 0.

P.S If you are not developing for embedded systems or obsolete consoles like the Gameboy Color (notice my gravatar), I strongly recommend you to use the const keyword rather than #defines for these kind of things.

5
votes

The problem is that you use macros. Your

#define LOCK_BYTES LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

doesn't do what you think. What it does is to textually replace every occurance of LOCK_BYTES with LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES. So

0 * LOCK_BYTES

expands to

0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES

This is C++. Avoid macros whenever possible. We have const for that.


This works just fine for me:

#include <iostream>

const int name_bytes = 256;
const int version_bytes = 256;
const int size_bytes = 32;
const int used_lock_count_bytes = 32;
const int lock_name_bytes = 256;
const int lock_type_bytes = 1;
const int pid_bytes = 4;
const int tid_bytes = 4;
const int lock_bytes = lock_name_bytes + lock_type_bytes + pid_bytes + tid_bytes;
const int header_bytes = name_bytes + version_bytes + size_bytes + used_lock_count_bytes;

int main() {
  std::cout << "lock_bytes: " << lock_bytes << std::endl;
  std::cout << "header_bytes: " << header_bytes << std::endl;
  std::cout << "lock_bytes * 0: " << 0 * lock_bytes << std::endl;
}

Do you have a good C++ book to learn from? You should.

2
votes

std::cout << "LOCK_BYTES * 0: " << 0 * LOCK_BYTES << std::endl;

expands to

std::cout << 0 * LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES << std::endl;

which, from the basic order of operations, is not equivalent to 0 * (that whole thing). Always enclose expressions inside macro definitions in parenthesis to avoid this sort of error — remember that preprocessor expands macros (more or less) literally.

1
votes

Posting for completeness:

const unsigned NAME_BYTES = 256;
const unsigned VERSION_BYTES = 256;
const unsigned SIZE_BYTES = 32;
const unsigned USED_LOCK_COUNT_BYTES = 32;
const unsigned LOCK_NAME_BYTES = 256;
const unsigned LOCK_TYPE_BYTES = 1;
const unsigned PID_BYTES = 4;
const unsigned TID_BYTES = 4;
const unsigned LOCK_BYTES = LOCK_NAME_BYTES + LOCK_TYPE_BYTES + PID_BYTES + TID_BYTES;
const unsigned HEADER_BYTES = NAME_BYTES + VERSION_BYTES + SIZE_BYTES + USED_LOCK_COUNT_BYTES;

Macros are expanded, consts aren't. Always prefer consts as they are type safe and don't have paren problems.