Back to For Beginners

C memory leak error

For Beginners

Started by zaydenam July 23, 2007 04:43 PM

14 comments, last by Mike.Popoloski 16 years, 8 months ago

zaydenam

122

Author

July 23, 2007 04:43 PM

Ive made a simple class to generate the HCF of an array of numbers number given by the user. Whenever I input more than 10 numbers to find the HCF of it generates the HCF but after it says press any key to continue, Windows pops up with a this program has encountered a problem and needs to be shut down. I've gone through my whole code but I can't find the problem. Can you guys help? Thanks in adance. Zayd Here's the header file:


#ifndef HCF_H
#define HCF_H

/*
 Highest Common Factor Class
 */
class HCF
{  
    	
public:

    unsigned long buffer[];
    unsigned long HCFans;
    int MAX_FACTORS;
    int index;
    
    HCF();
    ~HCF();		
		
	int addFactor(unsigned long);
	unsigned long getHCF();
	unsigned long euclidHCF(unsigned long, unsigned long); 
};

#endif

The class cpp file:


#include "hcf.h"
 

HCF::HCF()
{
	HCFans = 1;
    index = 0;
    const int MAX_FACTORS = 25;
    unsigned long buffer[MAX_FACTORS];    
}


HCF::~HCF()
{
}

int HCF::addFactor(unsigned long factor)
{
    if (index <= MAX_FACTORS)
    {                    
       unsigned long prevIndex = index;
       buffer[index] = factor; 
       index = prevIndex + 1;
       return 1;
    }
    
    if (index >= MAX_FACTORS)
    {
       return 0;   
    }
    
}

unsigned long HCF::getHCF()
{
   unsigned long euclidAns;
   
   euclidAns = euclidHCF(buffer[0], buffer[1]);
   
   for (int i = 0; i < index; i++)
       euclidAns = euclidHCF(euclidAns, buffer);
                            
   return euclidAns;
}

unsigned long HCF::euclidHCF(unsigned long a, unsigned long b)
{
    if(b == 0)
       return a;
       
    else
       return euclidHCF(b, (a % b));
}

And the main.cpp file



#include <cstdlib>
#include <iostream>
#include <stdlib.h>

#include "hcf.h"

using namespace std;

int main(int argc, char *argv[])
{
    
    HCF h1;
    h1.addFactor(1000);
    h1.addFactor(432322);
    h1.addFactor(432295);
    h1.addFactor(20400);
    h1.addFactor(5000);
    h1.addFactor(343223400);
    h1.addFactor(500143304);
    h1.addFactor(50423200);
    h1.addFactor(50003423);
    h1.addFactor(500042636);
    h1.addFactor(500042141);
    h1.addFactor(500042243);
    h1.addFactor(500042234);
    h1.addFactor(500042334);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042334);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    h1.addFactor(500042234);
    
    
    cout << h1.getHCF() << endl;
    
    h1.~HCF();

    system("PAUSE");
    return 0;
}

Palidine

1,315

July 23, 2007 04:48 PM

if (index <= MAX_FACTORS)

and any other similar occurrences I may have missed should be:

if (index < MAX_FACTORS)

In an array that is:

VarType array[SIZE];

SIZE is not a valid array index. the last valid array index is SIZE-1, since C arrays are zero based: i.e. if array[0] is a valid index then in order for it to contain exactly SIZE elemsnts, then the last index must be SIZE-1

example:

int myArray[5];//this is out of boundsmyArray[5];//this is notmyArray[4];

-me

Mike.Popoloski

3,259

July 23, 2007 04:48 PM

Well, I am not quite sure how much memory is getting allocated by

unsigned long buffer[]

as a member of your class, but I do know (I think) that your constructor isn't doing what you think it is doing.

Your constructor is simply declaring a local variable named buffer with a size of MAX_FACTORS. The class's variable buffer isn't getting initialized anywhere, so you are probably writing into critical memory, which can cause Windows to puke all over you.

Use std::vector. It will make your life 100000000 times easier.

EDIT: Palidine is also correct: you are indexing one too many elements of your array. But my original analysis still stands. Unless I am horribly mistaken about the use of declaring a variable without a size for the array. In my experience, you either declare an array with a fixed size, or declare a pointer and create the array later.

Mike Popoloski | Journal | SlimDX

zaydenam

122

Author

July 23, 2007 04:50 PM

Thanks for the fix, That woulve created problems for me later on.
BUt even no my array is smaller then the maximum size and I'm still getting the same errors.

Palidine

1,315

July 23, 2007 04:53 PM

oh yeah, as Mike points out, you're never allocating memory for the array.

declare buffer like this:

unsigned long *buffer;

in the constructor:

buffer = new unsigned long[MAX_FACTORS];

then in the destructor:

delete[] buffer;

-me

Mike.Popoloski

3,259

July 23, 2007 05:02 PM

Quote:Original post by Palidine
oh yeah, as Mike points out, you're never allocating memory for the array.

declare buffer like this:

*** Source Snippet Removed ***

in the constructor:
*** Source Snippet Removed ***

then in the destructor:
*** Source Snippet Removed ***

-me

Or..... std::vector!!!! Where is Zahlman when you need him... :)

std::vector<long> numbers;int HCF::addFactor(unsigned long factor){    numbers.push_back( factor );}

Hurrah! Now we don't need any maximum size! We can do any number of elements we want.

Mike Popoloski | Journal | SlimDX

zaydenam

122

Author

July 23, 2007 05:07 PM

Thanks alot for your help. I'm going to ead up mre about vectors now but the person who I'm making this for as a favour, prefers that I use mostly C instead of C++.

rip-off

11,000

July 23, 2007 05:13 PM

Quote:Original post by zaydenam
Thanks alot for your help. I'm going to ead up mre about vectors now but the person who I'm making this for as a favour, prefers that I use mostly C instead of C++.

If its only a preference and the person can't do it them selves, then you should take the easier (and bug-resistant) route [smile].

Anyway, doesn't the fact that you are already using classes render this argument pointless as your code won't work in C anyway until you make an interface for it, and that interface would not need to expose std::vector.

blog | twitter

Kimani

104

July 23, 2007 05:24 PM

Quote:Original post by zaydenam
Thanks alot for your help. I'm going to ead up mre about vectors now but the person who I'm making this for as a favour, prefers that I use mostly C instead of C++.

Like rip-off said, classes don't exist in C, only C++.

But for reference sake, if you want to make it mostly C, you would use malloc and free instead of new and delete, like so:

unsigned long * buffer;...buffer = malloc( sizeof(long) * MAX_FACTORS ); // instead of new...free( buffer ); // instead of delete

If you're going to need to rewrite it for your friend so it's C and not C++, you can look into modules, which is a design pattern that is like objects but not. You basically just have a struct that holds all the data, and functions that service that struct, taking a pointer to one as well as whatever data you want to apply to it.

zaydenam

122

Author

July 23, 2007 05:31 PM

He actually doesnt know C++, only C, I'm trying to use C++ as little as possible, but my prgram is going to be so big that without classes I would have a nightmare.
So I'll briefly explain the things like classes and objects and anything else that is necessary. He only needs to understand how the program works.

C memory leak error

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

C memory leak error

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines