Still celebrating National IT Professionals Day with 3 months of free Premium Membership. Use Code ITDAY17


new fails in this program

Posted on 1998-08-07
Medium Priority
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;

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) ;         = 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,
                        640, /* width */
                        480, /* height */
                        NULL) ;

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

   while ( TRUE ) {

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

         if ( msg.message == WM_QUIT ) {

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

      } else {

         DrawSomething (hwnd);

   return msg.wParam ;

   switch ( iMsg ) {
   case WM_SIZE:
      cxClient = LOWORD (lParam);
      cyClient = HIWORD (lParam);
      return 0;

   case WM_DESTROY:
      PostQuitMessage (0) ;
      return 0 ;

      xMouse = LOWORD(lParam);
      yMouse = HIWORD(lParam);
      return 0;

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

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 ) {

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

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


Question by:alfredj
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 7
  • 4
  • 2

Expert Comment

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

Expert Comment

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

should be

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

Expert Comment

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!
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!


Author Comment

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.
LVL 22

Accepted Solution

nietod earned 600 total points
ID: 1169666
answer coming.
LVL 22

Expert Comment

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 ) {

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.


Expert Comment

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

Author Comment

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?
LVL 22

Expert Comment

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....

Author Comment

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!

LVL 22

Expert Comment

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.


>> 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.

Author Comment

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?

LVL 22

Expert Comment

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.

Featured Post

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

Question has a verified solution.

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

Errors will happen. It is a fact of life for the programmer. How and when errors are detected have a great impact on quality and cost of a product. It is better to detect errors at compile time, when possible and practical. Errors that make their wa…
Templates For Beginners Or How To Encourage The Compiler To Work For You Introduction This tutorial is targeted at the reader who is, perhaps, familiar with the basics of C++ but would prefer a little slower introduction to the more ad…
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
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.

670 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