Bug in my C++ program seems really strange. (Update on debugging progress)

[mike3]

Hi.

I seem to have made some progress on finding that bug in my program. I
deactivated everything in the bignum package that was used except for
the returning of BigFloat objects. I even crippled all the
constructors. So now all the operations and constructors that were
used do is just return BigFloats but no memory is actually accessed at
any point, nor is any allocated. However, when I reenable those parts
of the constructor that allocate memory, and _then_ enable the part of
the _de_structor that _frees_ the memory, the bug appears. It only
seems when I go to allocate the "digits" buffer with (in constructor)

digits = new DIGIT32[length];

and free with (in destructor)

delete digits;

or:

(allocate in constructor) digits = (DIGIT32
*)malloc(length*sizeof(DIGIT32));

(free in destructor) free(digits);

When I don't call free or delete, though, the crash does not appear.
But it does not seem to be something needing the memory and then
accessing it as I tried it with none allocated in the first place and
"digits" set to a dummy pointer, and there was no bug nor any attempt
to access the memory as that would crash the program. It only happens
when you allocate memory in the constructor and free it in the
destructor. Freeing it immediately after allocation, in the
constructor, does not result in the crash bug.

What gives, anyway?

 

[mike3]

Ooops! Sorry about the double posting. Darn,
Google crashed...

 

[Alf P. Steinbach]

* mike3:

Hi.

I seem to have made some progress on finding that bug in my program. I
deactivated everything in the bignum package that was used except for
the returning of BigFloat objects. I even crippled all the
constructors. So now all the operations and constructors that were
used do is just return BigFloats but no memory is actually accessed at
any point, nor is any allocated. However, when I reenable those parts
of the constructor that allocate memory, and _then_ enable the part of
the _de_structor that _frees_ the memory, the bug appears. It only
seems when I go to allocate the "digits" buffer with (in constructor)

digits = new DIGIT32[length];

and free with (in destructor)

delete digits;

or:

(allocate in constructor) digits = (DIGIT32
*)malloc(length*sizeof(DIGIT32));

(free in destructor) free(digits);

When I don't call free or delete, though, the crash does not appear.
But it does not seem to be something needing the memory and then
accessing it as I tried it with none allocated in the first place and
"digits" set to a dummy pointer, and there was no bug nor any attempt
to access the memory as that would crash the program. It only happens
when you allocate memory in the constructor and free it in the
destructor. Freeing it immediately after allocation, in the
constructor, does not result in the crash bug.

What gives, anyway?

This seems to be a classic case of violation of the rule of three. Rule
of three: if you have a destructor or an assignment operator or a copy
constructor, then you should in general have all three.

What happens with an RO3 violation is that an object is copied including
its pointers, in the raw, and then two or more objects all deallocate
via the same pointer values, so that something is multiply deallocated.

From another point of view an RO3 violation is most often a classic
case of evil premature optimization, usually going down to the lowest
possible level of abstraction instead of more sensibly using the highest
level available (like the standard C++ library), which provides the
required safe copying functionality automatically.

If I understand it correctly, what you have is C level code, let's call
that level 0, like

typedef ... DIGIT32; // Note: reserve all uppercase for macros!

struct MyClass
{
DIGIT32* myDigits;
int myLength;

MyClass( int aLength )
{
myLength = aLength;
digits = (DIGIT32)(malloc(...);
}

~MyClass()
{
free( myDigits );
}
};

There are number of abstraction levels between that hazardious C style
code and proper C++ code.

I'll discuss this in order, how to move from C style to C++.

Level 1, simply packaging that code using C++ constructs. A kind of
word for word translation from C to C++. This does not fix the RO3
violation, but paves the way for fixing it:

typedef ... Digit32;

class MyClass
{
private:
Digit32* myDigits;
int myLength;

public:
MyClass( int aLength )
: myLength( aLength )
, myDigits( new Digit32[aLength] )
{}

~MyClass() { delete[] myDigits; }
};

This does the same as the C-style code, but without casts and generally
in a bit more safe way, although as mentioned not fixing RO3 violation.

Level 2, implementing proper copying, that is, fixing that violation:

typedef ... Digit32;

class MyClass
{
private:
Digit32* myDigits;
int myLength;

public:
MyClass( int aLength )
: myLength( aLength )
, myDigits( new Digit32[aLength] )
{}

MyClass( MyClass const& other )
: myLength( other.myLength )
, myDigits( new Digit32[other.myLength] )
{
Digit32 const* const srcDigits = other.myDigits;
std::copy( srcDigits, srcDigits+myLength, myDigits );
}

~MyClass() { delete[] myDigits; }

void swap( MyClass& other )
{
std::swap( myDigits, other.myDigits );
std::swap( myLength, other.myLength );
}

MyClass& operator=( MyClass other )
{
swap( other );
return *this;
}
};

Now that became slightly complicated, so on to level 3, using standard
containers that implement the copying for you:

typedef ... Digit32;

class MyClass
{
private:
std::vector<Digit32> myDigits;
public:
MyClass( int aLength ): myDigits( aLength ) {}
};

Simple, yes?

Cheers, & hth.,

- Alf

 

[mike3]

* mike3:

Hi.

I seem to have made some progress on finding that bug in my program. I
deactivated everything in the bignum package that was used except for
the returning of BigFloat objects. I even crippled all the
constructors. So now all the operations and constructors that were
used do is just return BigFloats but no memory is actually accessed at
any point, nor is any allocated. However, when I reenable those parts
of the constructor that allocate memory, and _then_ enable the part of
the _de_structor that _frees_ the memory, the bug appears. It only
seems when I go to allocate the "digits" buffer with (in constructor)

digits = new DIGIT32[length];

and free with (in destructor)

delete digits;

(allocate in constructor) digits = (DIGIT32
*)malloc(length*sizeof(DIGIT32));

(free in destructor) free(digits);

When I don't call free or delete, though, the crash does not appear.
But it does not seem to be something needing the memory and then
accessing it as I tried it with none allocated in the first place and
"digits" set to a dummy pointer, and there was no bug nor any attempt
to access the memory as that would crash the program. It only happens
when you allocate memory in the constructor and free it in the
destructor. Freeing it immediately after allocation, in the
constructor, does not result in the crash bug.

What gives, anyway?

This seems to be a classic case of violation of the rule of three. Rule
of three: if you have a destructor or an assignment operator or a copy
constructor, then you should in general have all three.

What happens with an RO3 violation is that an object is copied including
its pointers, in the raw, and then two or more objects all deallocate
via the same pointer values, so that something is multiply deallocated.

That does not happen. See, I had in there a full copy constructor
that copied no pointers at all. Nor does the assignment operator
or any of those things copy _pointers_. They all copy _data_,
and only data. But in this case there isn't _anything_ going
on except blanks being constructed and passed around, as I've
stripped it down.

From another point of view an RO3 violation is most often a classic
case of evil premature optimization, usually going down to the lowest
possible level of abstraction instead of more sensibly using the highest
level available (like the standard C++ library), which provides the
required safe copying functionality automatically.

Actually there was no evil optimization.

If I understand it correctly, what you have is C level code, let's call
that level 0, like

typedef ... DIGIT32; // Note: reserve all uppercase for macros!

struct MyClass
{
DIGIT32* myDigits;
int myLength;

MyClass( int aLength )
{
myLength = aLength;
digits = (DIGIT32)(malloc(...);
}

~MyClass()
{
free( myDigits );
}
};

There are number of abstraction levels between that hazardious C style
code and proper C++ code.

I'll discuss this in order, how to move from C style to C++.

Level 1, simply packaging that code using C++ constructs. A kind of
word for word translation from C to C++. This does not fix the RO3
violation, but paves the way for fixing it:

typedef ... Digit32;

class MyClass
{
private:
Digit32* myDigits;
int myLength;

public:
MyClass( int aLength )
: myLength( aLength )
, myDigits( new Digit32[aLength] )
{}

~MyClass() { delete[] myDigits; }
};

That is more like what I have now, at this moment.
It is failing whenever delete[] digits is present.
When that is absent from the destructor and the
memory is allowed to "persist" (NOT something I'd
do in the real live program but something I'm doing
as part of my testing), the bug seems to go away.

This does the same as the C-style code, but without casts and generally
in a bit more safe way, although as mentioned not fixing RO3 violation.

Level 2, implementing proper copying, that is, fixing that violation:

The failure occurs even when the copy routines have
been specifically disabled and don't do a thing.

typedef ... Digit32;

class MyClass
{
private:
Digit32* myDigits;
int myLength;

public:
MyClass( int aLength )
: myLength( aLength )
, myDigits( new Digit32[aLength] )
{}

MyClass( MyClass const& other )
: myLength( other.myLength )
, myDigits( new Digit32[other.myLength] )
{
Digit32 const* const srcDigits = other.myDigits;
std::copy( srcDigits, srcDigits+myLength, myDigits );
}

~MyClass() { delete[] myDigits; }

void swap( MyClass& other )
{
std::swap( myDigits, other.myDigits );
std::swap( myLength, other.myLength );
}

MyClass& operator=( MyClass other )
{
swap( other );
return *this;
}
};

Now that became slightly complicated, so on to level 3, using standard
containers that implement the copying for you:

typedef ... Digit32;

class MyClass
{
private:
std::vector<Digit32> myDigits;
public:
MyClass( int aLength ): myDigits( aLength ) {}
};

Simple, yes?

Simple, yes. Can you access a "vector" like you do a
regular array? Ie. is myDigits[0] = 0xFFFFFFFF
something okay to do?

 

[Alf P. Steinbach]

* mike3:

Actually there was no evil optimization.

Using malloc and free is, just to give one example.

[snip]

Can you access a "vector" like you do a
regular array? Ie. is myDigits[0] = 0xFFFFFFFF
something okay to do?

Yes.

Although you'll most probably be better off using the 'at' member
function, because it checks whether the argument is a valid index.

Cheers, & hth.,

- Alf


PS: Please don't quote signatures.

 

[mike3]

* mike3:

Actually there was no evil optimization.

Using malloc and free is, just to give one example.

[snip]

But it's still buggered with new and delete, too. I
only went back to malloc and free to see if that
would resolve the bug, and it didn't.

By the way, if you need it, you can get the source
code to a "stripped down" version of the program
here:

http://www.mediafire.com/?cfmzd1y3yij

Can you access a "vector" like you do a
regular array? Ie. is myDigits[0] = 0xFFFFFFFF
something okay to do?

Yes.

Although you'll most probably be better off using the 'at' member
function, because it checks whether the argument is a valid index.

Alright.

Cheers, & hth.,

- Alf

PS: Please don't quote signatures.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

 

[Gianni Mariani]

http://www.mediafire.com/?cfmzd1y3yij

How does one build all that ?

 

[Alf P. Steinbach]

* Gianni Mariani:

How does one build all that ?

Changes to make it /compile/:


fracalg\computefrac.cpp(371):#if 0 //APS

fracalg\computefrac.cpp(378):#endif //APS

fracalg\computefrac.cpp(393):#if 0 //APS

fracalg\computefrac.cpp(396):#else //APS

fracalg\computefrac.cpp(397): err = FG3DError(FG3D_INVALID_FRACTAL_TYPE,
FractalType); //APS

fracalg\computefrac.cpp(398):#endif //APS
render\render.cpp(51): //APS MessageBox(NULL, TEXT("Zorg."),
TEXT(""), MB_OK);

render\render.cpp(69): //APS MessageBox(NULL, TEXT("Borg."),
TEXT(""), MB_OK);

win32\CMainWnd.cpp(52): wcx.hCursor = LoadCursor(0,
IDC_ARROW); //APS LoadCursor((HANDLE)NULL, IDC_ARROW); /* cursor */

win32\fg3dImageWindow.cpp(34): wcx.hCursor =
LoadCursor(0,IDC_ARROW); //APS //LoadCursor((HANDLE)NULL, IDC_ARROW); /*
cursor */

win32\fg3dNewImageWzrd.cpp(18)://APS HWND gTmp;

win32\fg3dNewImageWzrd.cpp(32)://APS gTmp = hwndTmp;

main.h(108)://APS extern HWND gTmp;

fracgen3d.cpp(79):// APS


Plus, the WinMain function must be changed to something like


int WINAPI WinMain(HINSTANCE TheInstance, HINSTANCE LastInstance,
LPSTR lpszCmdLine, int iCmdShow)
{
__try
{
return cppWinMain( TheInstance, LastInstance, lpszCmdLine,
iCmdShow );
}
__except(TRUE)
{
TCHAR szBuf[256];
StringCchPrintf(szBuf, 256, TEXT("EXCEPTION %08lX"),
GetExceptionCode());
OutputDebugString(szBuf);
return 0;
}
}


where cppWinMain contains the original code for that __try.

It's funny (is that the right word?) when the code contains C style
casts that makes it not compile, when all that's needed is to remove
those casts...

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).

Yes, I must be very bored to do such a thing!

Cheers, & hth. (although I didn't look at the silly bignum class,
whatever its fault is, it may just be corrupted memory in general),

- Alf

 

[BobR]

digits = new DIGIT32[length];

and free with (in destructor)

// > delete digits;

delete [] digits;

If you 'new', you 'delete'.
If you 'new[]', you 'delete[]'.

 

[mike3]

* Gianni Mariani:


Changes to make it /compile/:

fracalg\computefrac.cpp(371):#if 0 //APS

fracalg\computefrac.cpp(378):#endif //APS

fracalg\computefrac.cpp(393):#if 0 //APS

fracalg\computefrac.cpp(396):#else //APS

Excuse me, why all this? Those were commented
out for debugging purposes to help minimalize
the code.

fracalg\computefrac.cpp(397): err = FG3DError(FG3D_INVALID_FRACTAL_TYPE,
FractalType); //APS

fracalg\computefrac.cpp(398):#endif //APS

This too.

render\render.cpp(51): //APS MessageBox(NULL, TEXT("Zorg."),
TEXT(""), MB_OK);

render\render.cpp(69): //APS MessageBox(NULL, TEXT("Borg."),
TEXT(""), MB_OK);

Why couldn't it compile with those in?

win32\CMainWnd.cpp(52): wcx.hCursor = LoadCursor(0,
IDC_ARROW); //APS LoadCursor((HANDLE)NULL, IDC_ARROW); /* cursor */

win32\fg3dImageWindow.cpp(34): wcx.hCursor =
LoadCursor(0,IDC_ARROW); //APS //LoadCursor((HANDLE)NULL, IDC_ARROW); /*
cursor */

win32\fg3dNewImageWzrd.cpp(18)://APS HWND gTmp;

win32\fg3dNewImageWzrd.cpp(32)://APS gTmp = hwndTmp;

main.h(108)://APS extern HWND gTmp;

fracgen3d.cpp(79):// APS

Plus, the WinMain function must be changed to something like

int WINAPI WinMain(HINSTANCE TheInstance, HINSTANCE LastInstance,
LPSTR lpszCmdLine, int iCmdShow)
{
__try
{
return cppWinMain( TheInstance, LastInstance, lpszCmdLine,
iCmdShow );
}
__except(TRUE)
{
TCHAR szBuf[256];
StringCchPrintf(szBuf, 256, TEXT("EXCEPTION %08lX"),
GetExceptionCode());
OutputDebugString(szBuf);
return 0;
}

}

where cppWinMain contains the original code for that __try.

It's funny (is that the right word?) when the code contains C style
casts that makes it not compile, when all that's needed is to remove
those casts...

I must be using a really crappy C++ compiler then as
it allows that stuff.

So then it casts it automatically, you don't need all
that C-style stuff, then? Darn I'm too paranoid.

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).

That is uninitialized? It calls LockRectangle() right
in there @ line 46.

Yes, I must be very bored to do such a thing!

Cheers, & hth. (although I didn't look at the silly bignum class,
whatever its fault is, it may just be corrupted memory in general),

I've had a bear of a time trying to track it down.

 

[mike3]

digits = new DIGIT32[length];

and free with (in destructor)

// > delete digits;

delete [] digits;

If you 'new', you 'delete'.
If you 'new[]', you 'delete[]'.

Tried that, still crashes!

 

[BobR]

Excuse me, why all this? Those were commented
out for debugging purposes to help minimalize
the code.

Then they should have been removed for posting. I would be pissed if I spent
good money to download it with my slow dial-up.

Why couldn't it compile with those in?

It's windows code. GNU don't play that!

win32\CMainWnd.cpp(52): wcx.hCursor = LoadCursor(0,
IDC_ARROW); file://APS LoadCursor((HANDLE)NULL, IDC_ARROW); /* cursor */

win32\fg3dImageWindow.cpp(34): wcx.hCursor =
LoadCursor(0,IDC_ARROW); file://APS file://LoadCursor((HANDLE)NULL, IDC_ARROW); /*
cursor */

win32\fg3dNewImageWzrd.cpp(18)://APS HWND gTmp;
win32\fg3dNewImageWzrd.cpp(32)://APS gTmp = hwndTmp;
main.h(108)://APS extern HWND gTmp;
fracgen3d.cpp(79):// APS

Plus, the WinMain function must be changed to something like

int WINAPI WinMain(HINSTANCE TheInstance, HINSTANCE LastInstance,
LPSTR lpszCmdLine, int iCmdShow){
__try{
return cppWinMain( TheInstance, LastInstance, lpszCmdLine,
iCmdShow );
}
__except(TRUE){
TCHAR szBuf[256];
StringCchPrintf(szBuf, 256, TEXT("EXCEPTION %08lX"),
GetExceptionCode());
OutputDebugString(szBuf);
return 0;
}
}

where cppWinMain contains the original code for that __try.

It's funny (is that the right word?) when the code contains C style
casts that makes it not compile, when all that's needed is to remove
those casts...

I must be using a really crappy C++ compiler then as
it allows that stuff.

Then set it up for standard compliance, and raise the warning levels.

So then it casts it automatically, you don't need all
that C-style stuff, then? Darn I'm too paranoid.

If you must, use the C++ casts so you get the proper warnings/errors.

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).

That is uninitialized? It calls LockRectangle() right
in there @ line 46.

Did you check whether the pointer is valid during/after the 'call'?

If nothing else, try something like:
std::cout<<"pointer addr="<<pointer<<" calling:"<<*pointer;

If the pointer is invalid, that should cause a 'crash'. Then you'll know.

I've run into trouble in my own "spaghetti" code. Now days I separate the
GUI interface, C++ interface, and the work-horse code. Makes it much easier
to track problems, port to another OS, etc.. ( even wxWidgets needs some
'tweaking' between windows and GNU (GTK), but the underlying std C++ code
stays the same.)

 

[mike3]

Then they should have been removed for posting. I would be pissed if I spent
good money to download it with my slow dial-up.

Looking again at the changes he posted I'm not sure
what they were supposed to do, if they were to
remove undefines or add them in. I looked at my
package and there doesn't seem to be anything
commented out or otherwise removed. Perhaps he
could explain the problem that this was supposed
to remedy? I'm not sure what is going on there
now.

It's windows code. GNU don't play that!

He was using GNU compilers? I thought there
were versions of the GNU compilers for Windows.
If his compiler, GNU or otherwise, was unable
to support Windows he couldn't have run the
program as the whole "main" thing, "wnd" stuff
was all Windows. I'm guessing his compiler did
support Windows since he was able to run it
without modifications _that_ extensive.

Then set it up for standard compliance, and raise the warning levels.

What compiler do you use, by the way? I've got a
version of the GNU compilers on my system too,
and I could feed the non-Windows stuff through
it. If you're using GNU compilers (as I've
assumed from your "GNU don't play that!" comment),
could you explain how you do this (the "set it up
for standard compilance" part. I already know
how to raise the warning level.).

If you must, use the C++ casts so you get the proper warnings/errors.

Well I've been converting my main program to use
these.

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).

That is uninitialized? It calls LockRectangle() right
in there @ line 46.

Did you check whether the pointer is valid during/after the 'call'?

Why the scare quotes around "call", anyway?

 

[Alf P. Steinbach]

* mike3:

Excuse me, why all this? Those were commented
out for debugging purposes to help minimalize
the code.

I don't remember but I think you can figure it out.

This too.

This I think I remember. You called some fractal computation functions
that you didn't supply.

Why couldn't it compile with those in?

Would compile but would not work, just silent termination. Don't set up
message loops in rendering. Btw., that has nothing to do with C++.

win32\CMainWnd.cpp(52): wcx.hCursor = LoadCursor(0,
IDC_ARROW); //APS LoadCursor((HANDLE)NULL, IDC_ARROW); /* cursor */

win32\fg3dImageWindow.cpp(34): wcx.hCursor =
LoadCursor(0,IDC_ARROW); //APS //LoadCursor((HANDLE)NULL, IDC_ARROW); /*
cursor */

win32\fg3dNewImageWzrd.cpp(18)://APS HWND gTmp;

win32\fg3dNewImageWzrd.cpp(32)://APS gTmp = hwndTmp;

main.h(108)://APS extern HWND gTmp;

fracgen3d.cpp(79):// APS

Plus, the WinMain function must be changed to something like

int WINAPI WinMain(HINSTANCE TheInstance, HINSTANCE LastInstance,
LPSTR lpszCmdLine, int iCmdShow)
{
__try
{
return cppWinMain( TheInstance, LastInstance, lpszCmdLine,
iCmdShow );
}
__except(TRUE)
{
TCHAR szBuf[256];
StringCchPrintf(szBuf, 256, TEXT("EXCEPTION %08lX"),
GetExceptionCode());
OutputDebugString(szBuf);
return 0;
}

}

where cppWinMain contains the original code for that __try.

It's funny (is that the right word?) when the code contains C style
casts that makes it not compile, when all that's needed is to remove
those casts...

I must be using a really crappy C++ compiler then as
it allows that stuff.

Yes. In addition it seem you're not using your compiler in the best
possible way. After upgrading to a better compiler, turn on all
standard-conformance and all warnings, make the thing compile cleanly.

So then it casts it automatically, you don't need all
that C-style stuff, then? Darn I'm too paranoid.

No, it's good to be paranoid in coding. But introducing casts is the
opposite of being paranoid: you're turning off all checks!

To be sufficiently paranoid you need to do the /opposite/: no casts.

Or at least as few as possible, and none in high-level code.

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).

That is uninitialized? It calls LockRectangle() right
in there @ line 46.

I'm just reporting what it did. There was no actual call to that
function at run-time, before the crash. As to why that should happen,
well.

I've had a bear of a time trying to track it down.

General advice: in lots of places in the code there are checks whether
something is initialized or not.

If you instead make sure that constructors either initialize completely
and successfully, or else throw exceptions, you will only have properly
initialized objects around, and so you won't have to do that complicated
and leg-breaking dance.

Where it seems that you absolutely need some halfway initialized state,
like, calling a common Init function, consider replacing that Init
function with a proper constructor in a private base class (of course
this most often indicates a design issue, but better with a technical
fix than having that halfway initialized state causing general mess).

Cheers, & hth.,

- Alf

 

[mike3]

* mike3:







I don't remember but I think you can figure it out.



This I think I remember. You called some fractal computation functions
that you didn't supply.

Darnit, I forgot to remove the references in
stripping. I'm updating my main program, by
the way, to take into account all these
things you've mentioned.

Why couldn't it compile with those in?

Would compile but would not work, just silent termination. Don't set up
message loops in rendering. Btw., that has nothing to do with C++.

OK.

win32\CMainWnd.cpp(52): wcx.hCursor = LoadCursor(0,
IDC_ARROW); //APS LoadCursor((HANDLE)NULL, IDC_ARROW); /* cursor */
win32\fg3dImageWindow.cpp(34): wcx.hCursor =
LoadCursor(0,IDC_ARROW); //APS //LoadCursor((HANDLE)NULL, IDC_ARROW); /*
cursor */
win32\fg3dNewImageWzrd.cpp(18)://APS HWND gTmp;
win32\fg3dNewImageWzrd.cpp(32)://APS gTmp = hwndTmp;
main.h(108)://APS extern HWND gTmp;
fracgen3d.cpp(79):// APS
Plus, the WinMain function must be changed to something like
int WINAPI WinMain(HINSTANCE TheInstance, HINSTANCE LastInstance,
LPSTR lpszCmdLine, int iCmdShow)
{
__try
{
return cppWinMain( TheInstance, LastInstance, lpszCmdLine,
iCmdShow );
}
__except(TRUE)
{
TCHAR szBuf[256];
StringCchPrintf(szBuf, 256, TEXT("EXCEPTION %08lX"),
GetExceptionCode());
OutputDebugString(szBuf);
return 0;
}
}
where cppWinMain contains the original code for that __try.
It's funny (is that the right word?) when the code contains C style
casts that makes it not compile, when all that's needed is to remove
those casts...

I must be using a really crappy C++ compiler then as
it allows that stuff.

Yes. In addition it seem you're not using your compiler in the best
possible way. After upgrading to a better compiler, turn on all
standard-conformance and all warnings, make the thing compile cleanly.

What would be a better compiler, by the way? I've only
got this one since I didn't have to pay money for it
and I do not have a lot of money.

No, it's good to be paranoid in coding. But introducing casts is the
opposite of being paranoid: you're turning off all checks!

To be sufficiently paranoid you need to do the /opposite/: no casts.

Or at least as few as possible, and none in high-level code.

OK.

Then I'd be curious to know. Does this require a cast,
to get the value of the pointer for our error information?:

FG3DError SomeRoutine(BigFloat *a)
{
FG3DError err;

<blah blah blah>

if(<some operation on "a" failed>)
return(FG3DError(<Error code>, a)); <--- here
else
return(FG3DError(FG3D_SUCCESS));
}

And this?

long MultiplyShorts(short b, short c)
{
long a;

a = b*c;

return(a);
}

Will this give the full "long" result or
do I need to typecast b and c to "short"?
And will it do that on all compilers (in case
I change my choice of compiler in the future),
ie. is that "standard" behavior? Especially
that last part right there -- is it _standard_
for a = b*c to be the full long product of b
and c, or not? I need that long product!

or this?:

double DivideInts(int a, int b)
{
double t;

t = a/b;

return(t);
}

Does that give a full floating-point double-
precision quotient of a and b or an integer one?
By the _C++ standard_, remember. See, in all
three of these situations I've used a cast, and
it is these situations that account for the vast
majority of my cast use.

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).

That is uninitialized? It calls LockRectangle() right
in there @ line 46.

I'm just reporting what it did. There was no actual call to that
function at run-time, before the crash. As to why that should happen,
well.

Uh oh...

Why wouldn't it call? That does not make any sense! It
calls on my machine.

General advice: in lots of places in the code there are checks whether
something is initialized or not.

I'm paranoid.

If you instead make sure that constructors either initialize completely
and successfully, or else throw exceptions, you will only have properly
initialized objects around, and so you won't have to do that complicated
and leg-breaking dance.

The constructors do throw exceptions if they cannot initialize
something
completely and successfully. So it would be OK to remove all
initialization
checks except the exception throwing, then?

Where it seems that you absolutely need some halfway initialized state,
like, calling a common Init function, consider replacing that Init
function with a proper constructor in a private base class (of course
this most often indicates a design issue, but better with a technical
fix than having that halfway initialized state causing general mess).

The reason I chose the "Init" function in the bignum "BigFloat"
thing was beacuse I would have to repeat the same memory-allocation
code in each constructor so I wanted to save some space. I heard
repetition of code wasn't a good idea, so I just encapuslated
that code which would be identical between the constructors
in a nice, easy-to-service routine called "Init". "Init"
is never called anywhere else except in constructors for
BigFloat (it's a "private" member function as well.). At no
point do I need a "half-initialized" something. Whenever I
need it initialized in a certain way, I just make another
constructor for that (For example there's a constructor that
will initialize a BigFloat to a given "double" float.).

That's all I have that for, to make maintenance easier and
to avoid repetition.

 

[Alf P. Steinbach]

* mike3:

What would be a better compiler, by the way? I've only
got this one since I didn't have to pay money for it
and I do not have a lot of money.

It seems you forgot to state which compiler you're using.

However, seeing as this is Windows programming, Visual C++ 8.0 is fairly
good and gratis. Including an IDE, which, unfortunately, is crappy.

[snip]

Then I'd be curious to know. Does this require a cast,
to get the value of the pointer for our error information?:

FG3DError SomeRoutine(BigFloat *a)
{
FG3DError err;

<blah blah blah>

if(<some operation on "a" failed>)
return(FG3DError(<Error code>, a)); <--- here
else
return(FG3DError(FG3D_SUCCESS));
}

Do something like

void someRoutine( BigFloat const& a )
{
// some operation on "a"
}

If "some operation" fails it will throw an exception.

Notice how extremely much simpler this is, in addition to be much more safe.

And this?

long MultiplyShorts(short b, short c)
{
long a;

a = b*c;

return(a);
}

Here you need conversion:

inline long productOf( short a, short b )
{
long aLong const = a;
return aLong*b;
}

or

inline long productOf( short a, short b )
{
return (a+0L)*b;
}

Since this function is evidently intended to be used in expressions, the
name has been changed to indicate the result rather than the operation
(like, instead of "computeSin", just "sin").

Will this give the full "long" result

No, in the code you showed the arguments would be promoted to int, the
computation done as int, and only then the result converted to long.

or
do I need to typecast b and c to "short"?
?


[snip]
double DivideInts(int a, int b)
{
double t;

t = a/b;

return(t);
}

Does that give a full floating-point double-
precision quotient of a and b or an integer one?
Integer.


By the _C++ standard_, remember. See, in all
three of these situations I've used a cast, and
it is these situations that account for the vast
majority of my cast use.

No need to use casts in any of the code you've shown.

By not using casts needlessly, and using the C++ style casts (like
static_cast, reinterpret_cast, dynamic_cast, const_cast) where you
absolutely need one, you make it possible to /search/ for casts.

Searching for casts can be relevant because casts are a main source of
bugs -- which is the main reason to avoid them. ;-)

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).
That is uninitialized? It calls LockRectangle() right
in there @ line 46.

I'm just reporting what it did. There was no actual call to that
function at run-time, before the crash. As to why that should happen,
well.

Uh oh...

Why wouldn't it call? That does not make any sense! It
calls on my machine.

Checking...

This happens with menu choice "New fractal...".

Oops, I said something wrong (hasty me). LockRectangle /is/ called, but
the section of code there that initializes the "d3dlr", where I
naturally placed the breakpoint, is within

if(LockStatus == FALSE) { ... }

and LockStatus has a non-zero value, apparently uninitialized.

Summing up, I think it would really help if you tried very hard to do
/guaranteed/ initialization. Not calling Init-functions. Using
constructors that either initialize everything successfully, or throw.


[snip]

The constructors do throw exceptions if they cannot initialize
something
completely and successfully. So it would be OK to remove all
initialization
checks except the exception throwing, then?

Yes, when the other aspects are fixed. By that I mean, not calling
Init-functions that call other functions... Using just constructors,
and even introducing dummy private base classes to do this.

Now as mentioned that isn't ideal. It's just a technical fix. Ideal
would be some redesign.

But by limiting yourself to a very harsh initialization regime, you'll
avoid pitfalls that otherwise could and probably would strand you (until
you gain more experience with initialization concerns in C++).

Cheers, & hth.,

- Alf


PS: *PLEASE DON'T QUOTE SIGNATURES!*

 

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

What compiler do you use, by the way? I've got a
version of the GNU compilers on my system too,
and I could feed the non-Windows stuff through
it. If you're using GNU compilers (as I've
assumed from your "GNU don't play that!" comment),
could you explain how you do this (the "set it up
for standard compilance" part. I already know
how to raise the warning level.).

-std=c++98

 

[mike3]

* mike3:




It seems you forgot to state which compiler you're using.

I use the Borland C++ Builder 5.5 Command line tools.

However, seeing as this is Windows programming, Visual C++ 8.0 is fairly
good and gratis. Including an IDE, which, unfortunately, is crappy.

Gratis, really?

Is there a "stripped" version that has only the
C++ compilers, like with the Borland thing?

[snip]

Then I'd be curious to know. Does this require a cast,
to get the value of the pointer for our error information?:

FG3DError SomeRoutine(BigFloat *a)
{
FG3DError err;

<blah blah blah>

if(<some operation on "a" failed>)
return(FG3DError(<Error code>, a)); <--- here
else
return(FG3DError(FG3D_SUCCESS));
}

Do something like

void someRoutine( BigFloat const& a )
{
// some operation on "a"
}

If "some operation" fails it will throw an exception.

What if the operation though is something that returns
an error value instead? Oh and when I throw the exception
I also need to convert a pointer to give additional
information to whatever handler catches the exception,
(there's handlers in any area where exceptions may
get thrown out of) for example to show that an
operation on "a" failed.

Should there be _no_ functions that return error
values, and instead _always_ throw exceptions when
they fail (as that way you don't have to worry about
neglecting an error check around one, you can just
keep a few exception handlers at various levels,
etc.?)?

Notice how extremely much simpler this is, in addition to be much more safe.





Here you need conversion:

inline long productOf( short a, short b )
{
long aLong const = a;
return aLong*b;
}

or

inline long productOf( short a, short b )
{
return (a+0L)*b;
}

Since this function is evidently intended to be used in expressions, the
name has been changed to indicate the result rather than the operation
(like, instead of "computeSin", just "sin").

I actually do not have a function like that in my
program, but the situation shown in that minimal
function is often found in more complex functions
like the bignum routines, for example. So then
I'd need to create ad hoc variables specifically
for the purpose of converting?

No, in the code you showed the arguments would be promoted to int, the
computation done as int, and only then the result converted to long.

So then you have to typecast, right?

?

I meant, typecast to "long", sorry.

[snip]

double DivideInts(int a, int b)
{
double t;

t = a/b;

Does that give a full floating-point double-
precision quotient of a and b or an integer one?

Integer.

So then would a typecast be a good idea?

Or is does this do it, and is it "proper"?:

---
double DivideInts(int a, int b)
{
double adbl = a;
double bdbl = b;

return(adbl/bdbl);
}
---

(No typecasting)

No need to use casts in any of the code you've shown.

By not using casts needlessly, and using the C++ style casts (like
static_cast, reinterpret_cast, dynamic_cast, const_cast) where you
absolutely need one, you make it possible to /search/ for casts.

Searching for casts can be relevant because casts are a main source of
bugs -- which is the main reason to avoid them. ;-)

I've got nearly all the casts converted, so now I
can start removing them.

Having done all the above the program crashes on using an invalid
pointer txtrptr in [render.cpp] at line 62, which is due to using an
uninitialized member "d3dlr" (presumably there should have been an
earlier call to FractalImage::LockRectangle, but no such).
That is uninitialized? It calls LockRectangle() right
in there @ line 46.
I'm just reporting what it did. There was no actual call to that
function at run-time, before the crash. As to why that should happen,
well.

Uh oh...

Why wouldn't it call? That does not make any sense! It
calls on my machine.

Checking...

This happens with menu choice "New fractal...".

Oops, I said something wrong (hasty me). LockRectangle /is/ called, but
the section of code there that initializes the "d3dlr", where I
naturally placed the breakpoint, is within

if(LockStatus == FALSE) { ... }

and LockStatus has a non-zero value, apparently uninitialized.

Summing up, I think it would really help if you tried very hard to do
/guaranteed/ initialization. Not calling Init-functions. Using
constructors that either initialize everything successfully, or throw.

The structure, "FractalImage" has a real constructor,
but I only added the LockStatus flag in LATER, and FORGOT
to put it in the constructor! Dang!!!

It works now! The bug has been fixed!

The Init functions are *pnly* and I say again, *ONLY*
called in CONSTRUCTORS, period. And *always* called from
them. They're just there to save a bit of cutting-and-
pasting. At no point are they called anywhere else.

[snip]

The constructors do throw exceptions if they cannot initialize
something
completely and successfully. So it would be OK to remove all
initialization
checks except the exception throwing, then?

Yes, when the other aspects are fixed. By that I mean, not calling
Init-functions that call other functions... Using just constructors,
and even introducing dummy private base classes to do this.

Now as mentioned that isn't ideal. It's just a technical fix. Ideal
would be some redesign.

Could you tell me where my design is wanting, by the way?
I'd like to know so I can improve it.

But by limiting yourself to a very harsh initialization regime, you'll
avoid pitfalls that otherwise could and probably would strand you (until
you gain more experience with initialization concerns in C++).

I try to make sure it's all initialized, but as I add
more to the classes sometimes I forget to add something
(like the dilemma that caused this whole problem) to the
initializing procedures. That's called "human error",
and it's bound to happen eventually.

Cheers, & hth.,

- Alf

PS: *PLEASE DON'T QUOTE SIGNATURES!*

Signature snipped.

 

[Duane Hebert]

I use the Borland C++ Builder 5.5 Command line tools.

Gratis, really?

Is there a "stripped" version that has only the
C++ compilers, like with the Borland thing?

I think it just does c++ but it has an IDE. You can also get Borland's
Turbo
Explorer that's based on BDS2006. It has an IDE and a newer compiler
than 5.5. Or just get both and see what you like. Both are free.

 

[mike3]

Thanks.