Jump to content
  • Advertisement
Sign in to follow this  
zaydenam

C memory leak error

This topic is 4077 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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;
}

Share this post


Link to post
Share on other sites
Advertisement

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 bounds
myArray[5];

//this is not
myArray[4];




-me

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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++.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!