[Tango Issac Debian]

I facing a problem due to Segmentation Fault on a void pointer using in the code.

Here,

[CODE] struct Heap {
    
        void **ap;

   }hp;

In a Function I initializing this pointer ap as :
      if( ( hp->ap = (void**)malloc( c * sizeof( void * ) ) ) == NULL )
	return -1;[/CODE]

In this specific line I getting the Segmentation fault. I think this is due to the void pointer.

But , Please say me what is possible solution ?

[shabbir]

why sizeof(void*) ? and also whats c

[Tango Issac Debian]

This is Heap.h

#ifndef _HEAP_H_
#define _HEAP_H_

typedef int ( *heapcomparefunc )( void *p0, void *p1 );

typedef struct _heap {
    void **ap;
    unsigned int cp, cpAlloc;
    heapcomparefunc phcf;
} heap;

extern int HeapCreate( heap *ph, unsigned int c, heapcomparefunc phcf );
extern void HeapDestroy( const heap *ph );
extern int HeapInsert( heap *ph, void *p );
extern void *HeapDelete( heap *ph );
extern int HeapDeleteItem( heap *ph, const void *p );
extern void *HeapLookup( const heap *ph );

#endif

The implementation of the library of the Heap.h is here in the Heap.c

#include <stddef.h>
#include <stdlib.h>

#include "heap.h"

extern int HeapCreate( heap *ph, unsigned int c, heapcomparefunc phcf ) {

    if( ( ph->ap = (void**)malloc( c * sizeof( void * ) ) ) == NULL )
		return -1;

    ph->cp = 0;
    ph->cpAlloc = c;
    ph->phcf = phcf;
    
    return 0;
}

extern void HeapDestroy( const heap *ph ) {

    free( ph->ap );
}

extern int HeapInsert( heap *ph, void *p ) {

    unsigned int i;
    void *pTemp;
    
    if( ph->cp == ph->cpAlloc ) {
		if( ( ph->ap = (void**)realloc( ph->ap, ( ph->cpAlloc << 1 ) *
				 sizeof( void * ) ) ) == NULL )
			return -1;

		ph->cpAlloc <<= 1;
    }

    ph->ap[ ph->cp ] = p;

    for( i = ph->cp++; i && ph->phcf( ph->ap[ i ],
				      ph->ap[ ( i - 1 ) >> 1 ] ) < 0;
	 i = ( i - 1 ) >> 1 ) {
		pTemp = ph->ap[ i ];
		ph->ap[ i ] = ph->ap[ ( i - 1 ) >> 1 ];
		ph->ap[ ( i - 1 ) >> 1 ] = pTemp;
    }
    
    return 0;
}

static int DeleteItem( heap *ph, unsigned int i ) {

    void *pTemp;
    unsigned int iSwap;
    
    ph->ap[ i ] = ph->ap[ --ph->cp ];

    while( ( i << 1 ) + 1 < ph->cp ) {
		iSwap = i;
	
		if( ph->phcf( ph->ap[ iSwap ], ph->ap[ ( i << 1 ) + 1 ] ) > 0 )
			iSwap = ( i << 1 ) + 1;

		if( ( i << 1 ) + 2 < ph->cp &&
			ph->phcf( ph->ap[ iSwap ], ph->ap[ ( i << 1 ) + 2 ] ) > 0 )
			iSwap = ( i << 1 ) + 2;

		if( iSwap == i )
			break;

		pTemp = ph->ap[ i ];
		ph->ap[ i ] = ph->ap[ iSwap ];
		ph->ap[ iSwap ] = pTemp;
		
		i = iSwap;
    }

    if( ph->cpAlloc > 1 && ( ph->cp << 1 < ph->cpAlloc ) )
		return (( ph->ap = (void**)realloc( ph->ap, ( ph->cpAlloc >>= 1 ) *
				   sizeof( void * ) ) ) == NULL) ? 0 : -1;

    return 0;
}

extern int HeapDeleteItem( heap *ph, const void *p ) {

    unsigned int i;

    for( i = 0; i < ph->cp; i++ )
		if( ph->ap[ i ] == p )
			return DeleteItem( ph, i );

    return -1;
}

extern void *HeapDelete( heap *ph ) {

    void *p = HeapLookup( ph );

    if( !ph->cp )
		return NULL;

    return DeleteItem( ph, 0 ) ? NULL : p;
}

extern void *HeapLookup( const heap *ph ) {

    return ph->cp ? ph->ap[ 0 ] : NULL;
}

And, this is testing Main function.

#include "heap.h"
#include <stdio.h>
#include <string.h>

int rec_cmp (const char *a, const char *b)
 {

  return strcmp(a,b);
}

int main(int argc, char *argv[]){

  heap *heap;
   /*
    *I think the problem is here. When no nodes are present the *heap hold garbage   
    * Values  only. So, for allocating the Root Node we need to use malloc to initialize it?  
    */
   //Just to create the Root Node of the Heap 
  HeapCreate(heap, 1, (heapcomparefunc)rec_cmp);

}

So, where is the problem ??

[xpi0t0s]

heap is undefined and so ph->ap points somewhere random in memory. That's probably the problem.

Remember in C if you want to pass a variable to a function that will modify its value you have to pass the address of that variable. It's exactly the same with pointers; to initialise heap in HeapCreate you will need to pass in &heap which is of type heap **. And you WILL need to initialise heap before you use it.

[Tango Issac Debian]

you will need to pass in &heap which is of type heap **.

OK. This works. Instead of the

HeapCreate(heap, 1, (heapcomparefunc)rec_cmp);

The calling of the function of the HeapCreate(&heap, 1, (heapcomparefunc)rec_cmp);
is works.

And you WILL need to initialise heap before you use it.

Will I need to malloc for this structure heap sepratately in the main function? How it will be ?

[xpi0t0s]

Quote:

Originally Posted by Tango Issac Debian

Will I need to malloc for this structure heap sepratately in the main function? How it will be ?

Providing you put it between the definition and the first use it doesn't really matter where you put it. The statement will be something like:

heap=(heap*)malloc(sizeof(heap));

Of course, if you're simply doing

heap *heap;
heap=(heap*)malloc(sizeof(heap));

in the same function, unless there's a specific reason for using pointer semantics then you may as well replace this with

heap heap;

[Tango Issac Debian]

This modification helped me.

In the HeapTest.c

heap ph;

And,
Then calling the HeapCreate function as

HeapCreate(&hp, 1, (heapcomparefunc)rec_cmp);

Works Fine.

Sometime during testing I faced a very strange problem. That I seen the pointer addresses were holding the previus running time values, until I restarted the system. Any solution for that?

Thanks to you for helping me so much.

[imrantechi]

After restarting system how it's possible to keep same address...That's impossible.

[imrantechi]

It may be keep some garbage address. So after work finished by pointer just make it NULL.

[xpi0t0s]

Quote:

Originally Posted by Tango Issac Debian

Sometime during testing I faced a very strange problem. That I seen the pointer addresses were holding the previus running time values, until I restarted the system. Any solution for that?

Difficult to say without seeing some code (as in a minimal reproducing testcase). If pointers have the wrong values then probably you have a memory corruption somewhere. Specific pointer values are not predictable (except in very limited circumstances) so if everything is working fine then you shouldn't place any significance on any particular values.

For example in your original code you had:

ph->ap = (void**)malloc...

Now if this doesn't crash then it will modify the memory at &(ph->ap) which could belong to absolutely anything else in your program, there's no predicting what.

So a useful trick with pointers is to initialise them to NULL, then when they aren't needed any more and you delete the object they were pointing to, set them immediately back to NULL, and this way if you do use a dangling pointer then you'll get an immediate crash instead of some random memory address being corrupted.