Pi Benchmarking in C

I wrote the following program to calculate n digits of Pi (where n could be anything, like 10M) in order to benchmark the CPU and it works perfectly (without OpenMP):

```
/*
*
* Simple PI Benchmarking tool
* Author: Suyash Srijan
* Email: suyashsrijan@outlook.com
*
* This program calculates how much time your CPU takes to compute n digits of PI using Chudnovsky Algorithm
* (http://en.wikipedia.org/wiki/Chudnovsky_algorithm) and uses the GNU Multiple Precision Arithmetic Library
* for computation.
*
* For verification of digits, you can download the digits from here: http://piworld.calico.jp/estart.html
*
* It's a single threaded program but you can compile it with OpenMP support to enable parallelization.
* WARN: OpenMP support is experimental
*
* Compile using gcc : gcc -O2 -Wall -o pibench pibench.c -lgmp -lssl -lcrypto
* Compile using gcc (with OpenMP): gcc -O2 -Wall -o pibench pibench.c -lgmp -lssl -lcrypto -fopenmp
*
*/

#include
#include
#include
#include
#include
#include
#include
#include

/ Import OpenMP header if compiling with -fopenmp /
#if defined(_OPENMP)
#include
#endif

/ You can't compile this on Windows /
#ifdef _WIN32
#error >>> Fatal: It is not possible to compile this program on Windows <<<
#endif

/ Build timestamp /
#define build_time __TIME__
#define build_date __DATE__

/ Calculate log to the base 2 using GCC's bit scan reverse intrinsic /
__inline__ unsigned int clc_log2(const unsigned int num) {
return ((num <= 1) ? 0 : 32 - (__builtin_clz(num - 1)));
}

/ Calculate MD5 checksum for verification /
__inline__ char clc_md5(const char string) {
MD5_CTX context;
unsigned char digest[16];
char checksum = (char)malloc(33);
int i;

MD5_Init(&context);
MD5_Update(&context, string, strlen(string));
MD5_Final(digest, &context);

for (i = 0; i < 16; ++i) {
snprintf(&(checksum[i*2]), 3, "%02x", (unsigned int)digest[i]);
}
return checksum;
}

/* Calculate pi digits main functio

Just a few notes on some things I didn't see mentioned.

Compilation:

-
I originally couldn't compile the program with the command in the comments.


/tmp/cc2H2h0a.o: In function 'clc_pi':

test.c:(.text+0x148): undefined reference to 'clock_gettime'

test.c:(.text+0x2f0): undefined referenceto 'clock_gettime'

collect2: ld returned 1 exit status

Add -lrt to the list of libraries you link to.

// Compile using gcc : gcc -O2 -Wall -o pibench pibench.c -lgmp -lssl -lcrypto -lrt

Syntax:

-
The DEBUG stuff is distracting. Maybe it is temporary, but if you
wanted to leave it in, I suggest extracting it:

#include 

static inline void debug(const char *format, ...)
{
#ifdef DEBUG
    va_list ap;
    va_start(ap, format);
    vfprintf(stdout, format, ap);
    va_end(ap);
#endif
}

and calling it:

debug("Iteration %lu of %lu successfully executed\n", i, iters - 1);

If DEBUG is undefined, the inline debug function will be empty and will
be excluded during compilation - it disappears.

-
Put the else on its own line.

if (dd == 1) {
    FILE *file;
    if ((file = fopen("pidigits.txt", "w")) == NULL) {
        fprintf(stderr, "Error while opening file\n"); exit(-1); } else {
        fprintf(file, "%.1s.%s\n", digits_of_pi, digits_of_pi + 1);
        fclose(file); }
}

When you use it this way, it is very easy to overlook it. I almost glanced over it when examining your code. There isn't really a reason to put it on it's own line, except to save LOC, which you could do better in other places.

if (dd == 1) 
{
    FILE *file;
    if ((file = fopen("pidigits.txt", "w")) == NULL) 
    {
        fprintf(stderr, "Error while opening file\n"); exit(-1); 
    } else {
        fprintf(file, "%.1s.%s\n", digits_of_pi, digits_of_pi + 1);
        fclose(file); 
    }
}

-
Put all statements on separate lines. From Code Complete, 2nd Edition, pg. 759:


With statements on their own lines, the code reads from top to bottom, instead of top to bottom and left to right. When you’re looking for a specific line of code, your eye should be able to follow the left margin of the code. It shouldn’t have to dip into each and every line just because a single line might contain two statements.

-
I would use more comments, especially around your OpenMP #pragmas and function calls.

-
Define i in your for loops.(C99)

for (int i = 0x0; i < iters; i++)

Miscellaneous:

-
fopen(), a widely-used file I/O functions that you are using, got a facelift in C11. It now supports a new exclusive create-and-open mode (“...x“). The new mode behaves like O_CREAT|O_EXCL in POSIX and is commonly used for lock files. The “...x” family of modes includes the following options:

-
wx create text file for writing with exclusive access.

-
wbx create binary file for writing with exclusive access.

-
w+x create text file for update with exclusive access.

-
w+bx or wb+x create binary file for update with exclusive access.

Opening a file with any of the exclusive modes above fails if the file already exists or cannot be created. Otherwise, the file is created with exclusive (non-shared) access. Additionally, a safer version of fopen() called fopen_s() is also available. That is what I would use in your code if I were you, but I'll leave that up for you to decide and change.

-
CLOCK_REALTIME represents the machine's best-guess as to the current wall-clock, time-of-day time. This means that CLOCK_REALTIME can jump forwards and backwards as the system time-of-day clock is changed, including by NTP.

CLOCK_MONOTONIC represents the absolute elapsed wall-clock time since some arbitrary, fixed point in the past. It isn't affected by changes in the system time-of-day clock.

If you want to compute the elapsed time between two events observed on the one machine without an intervening reboot, CLOCK_MONOTONIC is the best option.

StackExchange Code Review Q#40256, answer score: 6