views:

204

answers:

2

Hi,

I'm currently learning libgmp and to that end I'm writing a small program which find prime factors. My program calls a function which fills an array with a varying amount of mpz_t integers, prime factors of a given number, which I need to return. I'm planning on setting the last element to NULL, so I know how many mpz_t integers the function found.

My problem is I'm getting double free errors with my array of pointers to mpz_t integers. I've written up some sample code illustrating my problem:

#include <stdlib.h>
#include <stdio.h>
#include <gmp.h>

int main(void)
{
    mpz_t *p = malloc(5*sizeof(mpz_t*));
    mpz_init_set_ui(p[0], 2UL);
    mpz_init_set_ui(p[1], 5UL);
    gmp_printf("%Zd %Zd\n", p[0], p[1]);
    mpz_clear(p[0]);
    mpz_clear(p[1]);
    free(p);
    return 0;
}

2 and 5 are printed to stdout, so allocation seems to be fine. But I'm getting the double free error below:

2 5
*** glibc detected *** ./lol: double free or corruption (out): 0x08e20020 ***
======= Backtrace: =========
/lib/libc.so.6(+0x6b6c1)[0xb77126c1]
/lib/libc.so.6(+0x6cf18)[0xb7713f18]
/lib/libc.so.6(cfree+0x6d)[0xb7716f8d]
/usr/lib/libgmp.so.3(__gmp_default_free+0x1d)[0xb77f53fd]
/usr/lib/libgmp.so.3(__gmpz_clear+0x2c)[0xb77ff08c]
./lol[0x80485e3]
/lib/libc.so.6(__libc_start_main+0xe6)[0xb76bdb86]
./lol[0x80484e1]

I'm still getting totally used to pointers, and gcc gives no errors, however I'm fairly sure this is wrong and I should be doing something like

mpz_init_set_ui(*p[0], 2UL);

instead of:

mpz_init_set_ui(p[0], 2UL);

But that gives me a compiler error

test.c:8: error: incompatible type for argument 1 of ‘__gmpz_init_set_ui’
/usr/include/gmp.h:925: note: expected ‘mpz_ptr’ but argument is of type ‘__mpz_struct’

Anyway, my questions are:

  1. I'm sure I should be dereferencing the pointer in the mpz_init_set_ui() call, why is that wrong?
  2. Is there a better way of doing this? Should I use a linked list?(I've not learned linked lists yet, I figure an array is best for this but if I'm really making things way more difficult, tell me) 3.Would it be better to create a struct with a pointer to my array and another variable with the amount of elements in my array and return a pointer to that instead?

The platform is linux 32-bit just in case that's relevant.

Here is the code I have just now, which I want to modify, I declare the array of mpz_t on the stack. But I want to make main() a function:

#include <stdio.h>
#include <stdlib.h>
#include "prime.h"

#define MAXFACTORS 100

int main(void)
{
    mpz_t numToFactor, factor;
    mpz_t result;/* used to pass return values from getPrimeFactor() */
    mpz_t primeFactors[MAXFACTORS];

    mpz_init_set_str(numToFactor, "18 446 744 073 709 551 615 436 457 568", 10);
    mpz_init(factor);
    mpz_init(result);

    int pFLen = 0;
    mpz_init(primeFactors[pFLen]);

    getPrimeFactor(numToFactor, result);
    mpz_set(factor, result);
    while(mpz_cmp_ui(factor, 0UL))
    {
        mpz_set(primeFactors[pFLen], factor);
        pFLen++;
        if(pFLen == MAXFACTORS)
        {
            puts("Ran out of space to store prime factors, quitting...");
        }
        mpz_init(primeFactors[pFLen]);

        mpz_divexact(factor, numToFactor, factor);
        mpz_set(numToFactor, factor);

        getPrimeFactor(factor, result);
        mpz_set(factor, result);
    }
    mpz_set(primeFactors[pFLen], numToFactor);
    pFLen++;

    int i;
    for(i = 0; i < pFLen; i++)
    {
        gmp_printf("%Zd ", primeFactors[i]);
    }
    puts("");

    mpz_clear(numToFactor);
    mpz_clear(factor);
    return 0;
}

Thanks in advance people,

+1  A: 

This line

 mpz_t *p = malloc(5*sizeof(mpz_t*));

is likely the cause of your troubles. You've allocated enough space for five pointers to mpz_ts, not for 5 mpz_ts. Depending on the size of an mpz_t, you could be writing past the end of the array, etc.

You'll want to say

 mpz_t *p = malloc(5*sizeof(mpz_t));

to allocate an array of 5 mpz_t's.

Logan Capaldo
I like to use the object itself for the argument to sizeof: `mpz_t *p = malloc(5 * sizeof p);` :-)
pmg
pmg, I do to, except you mean `sizeof *p` here, or else you have the same problem as the OP ;)
Logan Capaldo
I'll remember that, I never though to use the object. Never ceases to amaze me how complex code is, yet the syntax is pretty small. Thanks a bunch everyone!
teflon19
D'oh! Of course I meant `malloc(nelems * sizeof *p);`
pmg
+1  A: 

Just part of your question

mpz_t *p = ...;

p is a pointer to mpz_t; p[0] (same as *p) is a mpz_t, as is p[1] (same as *(p + 1)), ...

mpz_init_set_ui(*p[1], 5UL); /* error */

p[1] is a mpz_t. You cannot dereference it (I think).
You can use the following syntax if you prefer

mpz_init_set_ui(*(p + 1), 5UL);
pmg