Integer factoring in C using GMP for numbers

This code performs integer factoring using the GMP library, and can also use OpenMP for parallel computation.

I compile it using: gcc -lgmp -fopenmp and whatever optimization flags I wish to try.

```
#include
#include

void next_prime(MP_INT *x) {
MP_INT r;
mpz_init(&r);
if (!mpz_probab_prime_p(x, 128u)) {
mpz_mod_ui(&r, x, 2ul);
if (mpz_cmp_ui(&r, 0ul) == 0)
mpz_add_ui(x, x, 1ul);
else
mpz_add_ui(x, x, 2ul);
while (!mpz_probab_prime_p(x, 128u)) {
mpz_add_ui(x, x, 2ul);
}
}
mpz_clear(&r);
}

void prev_prime(MP_INT *x) {
MP_INT r;
mpz_init(&r);
if (!mpz_probab_prime_p(x, 128u)) {
mpz_mod_ui(&r, x, 2ul);
if (mpz_cmp_ui(&r, 0ul) == 0)
mpz_sub_ui(x, x, 1ul);
else
mpz_sub_ui(x, x, 2ul);
while (!mpz_probab_prime_p(x, 128u)) {
mpz_sub_ui(x, x, 2ul);
}
}
mpz_clear(&r);
}

unsigned long mp_log(MP_INT x, MP_INT base) {
MP_INT r;
mpz_init(&r);
unsigned long lg = 0;
mpz_set(&r, x);
if (base != NULL) {
while (mpz_cmp_ui(&r, 0ul) != 0) {
mpz_div(&r, &r, base);
lg += 1;
}
}
else {
while (mpz_cmp_ui(&r, 0ul) != 0) {
mpz_div_ui(&r, &r, 2ul);
lg += 1;
}
}
mpz_clear(&r);
return lg;
}

void factor(MP_INT x, MP_INT n, MP_INT *base) {
/ Initialize variables. /
MP_INT s;
MP_INT r;
mpz_init(&s);
mpz_init(&r);
/ Compute stuff. /
mpz_set(x, n);
if (mpz_probab_prime_p(n, 128u)) {
goto end;
}
mpz_set(&s, base);
mpz_mod(&r, n, &s);
if (mpz_cmp_ui(&r, 0ul) == 0) {
mpz_set(x, &s);
goto end;
}
unsigned long k = mp_log(n, base);
unsigned long a = mp_log(n, NULL);
if (mpz_cmp(x, n) == 0) {
/ Try different powers of n. /
MP_INT p;
mpz_init(&p);
mpz_pow_ui(&p, &

You can improve the readability of your code, your indentation is great but you have magic numbers and the variable names aren't very clear. You probably wouldn't need comments if your variable names were clear and you had symbolic constants rather than numbers.

Magic Numbers:

You have a lot of raw numbers : 65536, 10u, 128u, 1ul, 2ul ...

For example you can have a symbolic constant CHARACTER_BUFFER_SIZE for 65536. Using the gcc compiler you would write your constant as

#define CHARACTER_BUFFER_SIZE 65536

When compiling with g++ you would use

static const CHARACTER_BUFFER_SIZE = 65536;

this would change the first 2 lines in main to

char str[CHARACTER_BUFFER_SIZE];
    char bas[CHARACTER_BUFFER_SIZE];

If you use this number often, it makes it easier to change the value by just changing it where you define the constant. It also makes the code easier to read. The standard for symbolic constants in C and C++ is all caps with '_' between words.

Function Names:
Underscore in function names is a lot less common than it used to be, Camel Case is more common

next_prime => nextPrime
prev_prime => prevPrime

Variable Names:

The base variable name is almost understandable, but what kind of base, does it provide a starting point for the function, or is it a numeric base like Octal, Hex or Decimal?

It's really not clear from the code what x is.

In main() what are the variables str (almost obvious), bas, n, p, base?

Future Flexibility:

You main could be a little more flexible, currently you can only redirect input and a output, you could use command switches in your to select input and output files as well. In main it is possible to add a couple of FILE * variable to make it a little more general

FILE *inputFile = stdin;
    FILE *outputFile = stdout;

    while(fscanf(inputFile, "%s %s", str, bas) != EOF) {
        ...
        mpz_out_str(outputFile, 10u, &base);

StackExchange Code Review Q#124812, answer score: 3