Solved

new fails in this program

Posted on 1998-08-07
13
225 Views
Last Modified: 2010-05-18
Hi, and Thank You for considering this question.

I want to create an array of pointers to structure and allocate
the structures one-by-one in a for() loop.  This program fails
on or about the 19th iteration with an Access Violation while
executing "particle[p] = new PARTICLE"

I am using both 95 & NT4 and VC++5.

Here is the code. It is all here in one short file:

#include <windows.h>
#include <string.h>
#include <time.h>
#include <stdio.h>

typedef struct _PARTICLE_ {
   int id;
   int xPos;
   int yPos;
   int xDelta;
   int yDelta;
   int lifespan;
   int age;
   int red, green, blue;
   int oldX, oldY;
} PARTICLE;

LRESULT CALLBACK WndProc (HWND, UINT, WPARAM, LPARAM) ;
void DrawSomething (HWND hwnd);
static int cxClient, cyClient ;
static int xMouse, yMouse;

int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance,
                    PSTR szCmdLine, int iCmdShow)
{
   static char szAppName[] = "RandomWalk" ;
   HWND        hwnd ;
   MSG         msg ;
   WNDCLASSEX  wndclass ;

   wndclass.cbSize        = sizeof (wndclass) ;
   wndclass.style         = CS_HREDRAW | CS_VREDRAW ;
   wndclass.lpfnWndProc   = WndProc ;
   wndclass.cbClsExtra    = 0 ;
   wndclass.cbWndExtra    = 0 ;
   wndclass.hInstance     = hInstance ;
   wndclass.hIcon         = LoadIcon (NULL, IDI_APPLICATION) ;
   wndclass.hCursor       = LoadCursor (NULL, IDC_ARROW) ;
   wndclass.hbrBackground = (HBRUSH) GetStockObject (WHITE_BRUSH) ;
   wndclass.lpszMenuName  = NULL ;
   wndclass.lpszClassName = szAppName ;
   wndclass.hIconSm       = LoadIcon (NULL, IDI_APPLICATION) ;

   RegisterClassEx (&wndclass) ;

   hwnd = CreateWindow (szAppName,
                        "Random Walk",
                        WS_OVERLAPPEDWINDOW | WS_VSCROLL,
                        CW_USEDEFAULT,
                        CW_USEDEFAULT,
                        640, /* width */
                        480, /* height */
                        NULL,
                        NULL,
                        hInstance,
                        NULL) ;

   ShowWindow (hwnd, iCmdShow) ;
   UpdateWindow (hwnd) ;

   while ( TRUE ) {

      if ( PeekMessage (&msg, NULL, 0, 0, PM_REMOVE) ) {

         if ( msg.message == WM_QUIT ) {
            break;
         }

         TranslateMessage (&msg) ;
         DispatchMessage (&msg) ;

      } else {

         DrawSomething (hwnd);

      }
   }
   return msg.wParam ;
}

LRESULT CALLBACK WndProc (HWND hwnd, UINT iMsg, WPARAM wParam, LPARAM lParam)
{
   switch ( iMsg ) {
   
   case WM_SIZE:
      cxClient = LOWORD (lParam);
      cyClient = HIWORD (lParam);
      return 0;

   case WM_DESTROY:
      PostQuitMessage (0) ;
      return 0 ;

   case WM_MOUSEMOVE:
      xMouse = LOWORD(lParam);
      yMouse = HIWORD(lParam);
      return 0;
   }

   return DefWindowProc (hwnd, iMsg, wParam, lParam) ;
}

void
DrawSomething (HWND hwnd)
{
   HDC hdc;
   time_t theTime;

   const int maxParticles = 300;

   PARTICLE *particle[maxParticles];
   static int theEpoch;
   static int t;

   int  p = 0;

   if ( cxClient == 0 || cyClient == 0 ) {
      return;
   }

   /* initialize the particles */
   if ( theEpoch == 0 ) {
      theEpoch++;

      srand ((unsigned) time (&theTime));

      for ( p = 0; p < maxParticles; p++ ) {

       particle[p] = new PARTICLE;   /* Access violation here at 19th iteration */

         memset (particle[p], 0, sizeof(particle));

         particle[p]->id = p;

         particle[p]->xPos = cxClient / 2;
         particle[p]->yPos = cyClient / 2;

         particle[p]->xDelta = (rand() % 3) - 1;
         particle[p]->yDelta = (rand() % 3) - 1;

         particle[p]->lifespan = (rand() % 10) + 5;

         particle[p]->age = 0;

         particle[p]->red   = rand() % 256;
         particle[p]->green = rand() % 256;
         particle[p]->blue  = rand() % 256;

      }

      p = 0;
   }

   /* set the particle counter */
   p = t++ % maxParticles;

   /* paint the particle */
   hdc = GetDC (hwnd);

   ReleaseDC (hwnd, hdc);

}







0
Comment
Question by:alfredj
  • 7
  • 4
  • 2
13 Comments
 
LVL 5

Expert Comment

by:ecw
ID: 1169662
Try changing
    memset (particle[p], 0, sizeof(particle));
to
    memset (particle[p], 0, sizeof(PARTICLE));
0
 
LVL 22

Expert Comment

by:nietod
ID: 1169663
memset (particle[p], 0, sizeof(particle));

should be

memset (particle[p], 0, sizeof(PARTICLE));
0
 
LVL 22

Expert Comment

by:nietod
ID: 1169664
Opps, ecw, beat me to it.  His answer was not there when I started looking at the code.  He should get the points.

However to avoid this problem in the future, use a constructor.  That is what they are for!
0
Networking for the Cloud Era

Join Microsoft and Riverbed for a discussion and demonstration of enhancements to SteelConnect:
-One-click orchestration and cloud connectivity in Azure environments
-Tight integration of SD-WAN and WAN optimization capabilities
-Scalability and resiliency equal to a data center

 

Author Comment

by:alfredj
ID: 1169665
Okay great, that solved the problem.  I'll raise the points
to 150 and add one line to the program.

put "particle[p]->oldX = particle[p]->xPos;"
right after "p = t++ % maxParticles;"

and I get an Access Violation.
0
 
LVL 22

Accepted Solution

by:
nietod earned 150 total points
ID: 1169666
answer coming.
0
 
LVL 22

Expert Comment

by:nietod
ID: 1169667
This code has a bunch of problems.

The array
 PARTICLE *particle[maxParticles];
is local to a procedure and is not static, thus it is created and destroyed every time the procedure is called.  However, the procedure uses

  if ( theEpoch == 0 ) {
         theEpoch++;

to insure that the array is initialized only the first time (ever 2^32 times after that.)  That makes no sense.  Thus the 2nd time the procedure is called there is a new array with random data in it that does not get initialized.  That is the source of the current crash.

Another problem is that you allcoate the PARTICAL objects with new , but never delete them.

You need to take a different approach, but I'm not sure what is best as I'm not sure what your goal is.  A quick fix would be to make the array static, but then you would not know when to clean it up and you would have to redisign the code that prevents multiple initializions.  There are probably better fixes if you can explain the goal.

0
 
LVL 5

Expert Comment

by:ecw
ID: 1169668
Where is t set? Or do you rely on C++ initializing auto vars to 0?
0
 

Author Comment

by:alfredj
ID: 1169669
Thanks!  I'll go with the quick fix of putting "static" in front
of the array definition.

I don't really have a goal other than learning about VC++
and experimenting with things I read on the net and in
magazines.  Thanks for the help.  I'd love to give some points
to ecw (because he beat you to the answer by 5 mins) but your
second answer was what really helped me.

BTW, is there a way to SEARCH the database of old questions
by keyword?
0
 
LVL 22

Expert Comment

by:nietod
ID: 1169670
>> Where is t set? Or do you rely on C++ initializing auto vars to 0?

Strangely enough  that does't matter.  As far as I can tell, the code is used to randomly set items in the array so the initial value of "t" should not matter.

>>Thanks!  I'll go with the quick fix of putting "static" in front
>> of the array definition.
That leaves you with a memory leak.  I have no idea how you will fix it.  

Also you will need to redo the code that controls the initialization of the array.  Use a boolean instead of theEpoch to control the initialization.  The way it is now you increment TheEpoch each time the procedure is called.  Eventually it will rollover and become 0 again and then you will reinitialize the array again.  Use a static boolean that is set to false at first and then change it to true when the initialization is done.

Not that is not a good design!. But if that's what you want....
0
 

Author Comment

by:alfredj
ID: 1169671
I thought statics were always initialized to ZERO,
[K&R, 1st edition, page 37]
so in this case t is zero at the start
and theEpoch is zero at the start so the "then"
part of "if (theEpoch == 0) {" is only executed once and
will NOT rollover since 'theEpoch++' is inside the 'then'
clause. (Although I agree that I should use a boolean.)

't' is only used as the input to 't++ % maxParticles'
so it should not matter if it rolls over.

I know I have a memory leak, but since the only way out of
the 'while (TRUE) {' loop is to exit the program, it should
not be a big deal, but couldn't I use:

if (some_condition)
  for all particle[p]
    delete particle[p];

You guys are really great!  I really like this web community!
Thanks again!

0
 
LVL 22

Expert Comment

by:nietod
ID: 1169672
>> I thought statics were always initialized to ZERO,
Yes, for the built-in types unless explicily initialized to something else.

>>     part of "if (theEpoch == 0) {" is only executed once and
>>     will NOT rollover since 'theEpoch++' is inside the 'then'
>>     clause. (Although I agree that I should use a boolean.)

Yes, my mistake.

>>'t' is only used as the input to 't++ % maxParticles'
>>  so it should not matter if it rolls over.

correct.

>> know I have a memory leak, but since the only way out of
>> the 'while (TRUE) {' loop is to exit the program, it should
>> not be a big deal,

I'm not sure what the while loop has to do with anything.  Unless your points is that the memory will always be needed while the program is running. However it is a bad practice to not delete all remaining allocated items when the program terminates.  That is a way around some of the safegaurds C++ puts in to try to help you write safer code.

>> but couldn't I use:
>>
>>     if (some_condition)
>>       for all particle[p]
>>         delete particle[p];

Yes, but that would have to be done inside of DrawSomething().   You would have to pass some sort of semaphore to DrawSomething() to indicate that it should deleted instead of draw.  Doesn't that seem a little weird?.  Better to use a global array and have seperate functions for initializing, drawing, and deleting.
0
 

Author Comment

by:alfredj
ID: 1169673
Wow!  Thanks!  I really like this!  I don't get to
talk in this much detail about computer programs
very often.  This is a fantastic web site!

You seem like you have used this site a lot.  Do you know if
I can I search other people's questions by keyword?  Or is
the chronological list the only way to go?

0
 
LVL 22

Expert Comment

by:nietod
ID: 1169674
There is no search feature.  They were going to add one soon when I started here a year ago.  There are still going to add one soon.
0

Featured Post

Free Tool: IP Lookup

Get more info about an IP address or domain name, such as organization, abuse contacts and geolocation.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
White board coding practice 3 92
Error creating a new C++ project in ,net 20 42
Embarcadero C++ builder XE10.1 Berlin TRegistry declaration 1 41
C++ Code Issue 4 26
When writing generic code, using template meta-programming techniques, it is sometimes useful to know if a type is convertible to another type. A good example of when this might be is if you are writing diagnostic instrumentation for code to generat…
Introduction This article is a continuation of the C/C++ Visual Studio Express debugger series. Part 1 provided a quick start guide in using the debugger. Part 2 focused on additional topics in breakpoints. As your assignments become a little more …
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn additional member functions of the vector class. Specifically, the capacity and swap member functions will be introduced.

820 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question