newbee: trouble w/ try{} catch{}

There is a tool that we use here at work. The guy that wrote it has since
quit and I was asked if I would like to take it over (I'm learning to
program and the co. supports that) with that said, the code is really messy
and rather than fix ir "correct" i have started to design a new version from
the ground up, I am still responsible for keeping the old one running until
I roll out the new one.

OK, so I have a crash that happens randomly and I have asked the people here
to send me their logs when they crash, I have narrowed it down to one
operation and I don't understand why it's happening. I just thought I could
wrap it in a try/catch and handle it from there.

Here is the code:
inline byte GetRecordTypeId(void* pV)
{
if(pV)
{
try
{
return *(byte*)pV;
}
catch(char* e)
{
theLog.WriteError(e, __FILE__, __LINE__);
}
}
return kRecordTypeUndefined;
}


However, this appears to do nothing. It still crashes just the same as if
there was no try block. From what I understand, the above code should have
worked fine. What am I missing?

For testing purpose I forced a crash in the code by setting the void* to an
int (pV = (void*)someInt;)

Any help or suggestions would be great.

Thanks!

Jochen
Tue Sep 28 12:46:11 CDT 2004

Hi SteveK,

> However, this appears to do nothing. It still crashes just the same
> as if there was no try block. From what I understand, the above code
> should have worked fine. What am I missing?

You have missed to tell us the reason of the crash!
The best is to write a minidump when the crash occurs, so you can later
debug it (and then tell us the exception message).

--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/

Steve
Tue Sep 28 12:51:04 CDT 2004

>
> Here is the code:
> inline byte GetRecordTypeId(void* pV)
> {
> if(pV)
> {
> try
> {
> return *(byte*)pV;
> }
> catch(char* e)
> {
> theLog.WriteError(e, __FILE__, __LINE__);
> }
> }
> return kRecordTypeUndefined;
> }
>
>
catch(char* e) isn't a great way of catching the error, as you've found. If
you just want to try to trap everything, use catch(...) Your code is
probably just bugging out when the cast fails. This isn't great C++
programming :-)
Apart from that, if you can't redesign the code (and it sounds like
rebuilding is a good idea) you're better using a dynamic_cast - Google for
it. Make sure you enable RunTime Type Info in the project properties (I
think it's Compiler/Advanced or possibly Compiler/Language), and try (this
is uncompiled, BTW):

byte *b = dynamic_cast<byte *>( pV );
if ( b != NULL )
{
return *b;
}
else
{
return kRecordTypeUndefined;
}

SteveK
Tue Sep 28 12:57:19 CDT 2004

right you are, sorry about that!

"Unhandled exception at 0x00466bf7 in Moves2.exe: 0xC0000005: Access
violation reading location 0x0000029a."






"Jochen Kalmbach" <nospam-Jochen.Kalmbach@holzma.de> wrote in message
news:Xns9572C919962A9nospamJochenKalmbach@207.46.248.16...
> Hi SteveK,
>
> > However, this appears to do nothing. It still crashes just the same
> > as if there was no try block. From what I understand, the above code
> > should have worked fine. What am I missing?
>
> You have missed to tell us the reason of the crash!
> The best is to write a minidump when the crash occurs, so you can later
> debug it (and then tell us the exception message).
>
> --
> Greetings
> Jochen
>
> My blog about Win32 and .NET
> http://blog.kalmbachnet.de/

SteveK
Tue Sep 28 13:05:24 CDT 2004

Hi Steve,

Thanks for the quick response. I quickly tried to implement your sample and
received compiler errors:
"error C2680: 'byte *' : invalid target type for dynamic_cast target type
must be a pointer or reference to a defined class file.cpp"

"error C2680: 'byte *' : invalid target type for dynamic_cast"


So I will start the google process to see what I can find, but in the
meantime.. does this make any sense?






"Steve McLellan" <sjm.NOSPAM AT fixerlabs DOT com> wrote in message
news:ecF3zOYpEHA.1712@tk2msftngp13.phx.gbl...
> >
> > Here is the code:
> > inline byte GetRecordTypeId(void* pV)
> > {
> > if(pV)
> > {
> > try
> > {
> > return *(byte*)pV;
> > }
> > catch(char* e)
> > {
> > theLog.WriteError(e, __FILE__, __LINE__);
> > }
> > }
> > return kRecordTypeUndefined;
> > }
> >
> >
> catch(char* e) isn't a great way of catching the error, as you've found.
If
> you just want to try to trap everything, use catch(...) Your code is
> probably just bugging out when the cast fails. This isn't great C++
> programming :-)
> Apart from that, if you can't redesign the code (and it sounds like
> rebuilding is a good idea) you're better using a dynamic_cast - Google for
> it. Make sure you enable RunTime Type Info in the project properties (I
> think it's Compiler/Advanced or possibly Compiler/Language), and try (this
> is uncompiled, BTW):
>
> byte *b = dynamic_cast<byte *>( pV );
> if ( b != NULL )
> {
> return *b;
> }
> else
> {
> return kRecordTypeUndefined;
> }
>
>

Doug
Tue Sep 28 13:06:50 CDT 2004

SteveK wrote:

>There is a tool that we use here at work. The guy that wrote it has since
>quit and I was asked if I would like to take it over (I'm learning to
>program and the co. supports that) with that said, the code is really messy
>and rather than fix ir "correct" i have started to design a new version from
>the ground up, I am still responsible for keeping the old one running until
>I roll out the new one.
>
>OK, so I have a crash that happens randomly and I have asked the people here
>to send me their logs when they crash, I have narrowed it down to one
>operation and I don't understand why it's happening. I just thought I could
>wrap it in a try/catch and handle it from there.
>
>Here is the code:
>inline byte GetRecordTypeId(void* pV)
>{
> if(pV)
> {
> try
> {
> return *(byte*)pV;
> }
> catch(char* e)
> {
> theLog.WriteError(e, __FILE__, __LINE__);
> }
> }
> return kRecordTypeUndefined;
>}
>
>
>However, this appears to do nothing. It still crashes just the same as if
>there was no try block. From what I understand, the above code should have
>worked fine. What am I missing?

Exceptions don't work like that. If dereferencing pV crashes, it's
generating a Windows structured exception, not a C++ exception, and
certainly not a char*. People sometimes (usually wrongly) want to catch
structured exceptions in catch(...), and for more on that, see:

http://members.cox.net/doug_web/eh.htm

Note also that even if you were to catch the exception, the __FILE__ and
__LINE__ usage probably wouldn't help you much, because they reflect the
file and line in which they appear. Your problem appears to be one of
garbage input to GetRecordTypeId, and what you have tried won't tell you
where the call came from. IOW, the above is not giving you any new
information, as you already seem to know it's crashing there. In the
debugger, you could easily determine who called GetRecordTypeId by looking
in the call stack window. If you don't know how to use the minidump Jochen
mentioned (I confess I don't know how to do this - any tips, Jochen?), and
you can't otherwise debug the crash, you can always search your source code
for GetRecordTypeId. If that function is called from just a few places, you
might be able to spot the problem without too much trouble. In addition, you
can always instrument your code with OutputDebugString calls and view the
output using Sysinternals DbgView:

http://www.sysinternals.com/ntw2k/freeware/debugview.shtml

--
Doug Harrison
Microsoft MVP - Visual C++

Doug
Tue Sep 28 13:08:35 CDT 2004

"Steve McLellan" <sjm.NOSPAM AT fixerlabs DOT com> wrote:

>catch(char* e) isn't a great way of catching the error, as you've found. If
>you just want to try to trap everything, use catch(...)

No. See my other message for the why. :)

>Your code is
>probably just bugging out when the cast fails. This isn't great C++
>programming :-)
>Apart from that, if you can't redesign the code (and it sounds like
>rebuilding is a good idea) you're better using a dynamic_cast - Google for
>it. Make sure you enable RunTime Type Info in the project properties (I
>think it's Compiler/Advanced or possibly Compiler/Language), and try (this
>is uncompiled, BTW):
>
>byte *b = dynamic_cast<byte *>( pV );
>if ( b != NULL )
>{
> return *b;
>}
>else
>{
> return kRecordTypeUndefined;
>}
>

You can't use the dynamic_cast operator for that.

--
Doug Harrison
Microsoft MVP - Visual C++

Vladimir_petter
Tue Sep 28 13:17:37 CDT 2004


"SteveK" <asasd@asdfasdfsd.com> wrote in message
> OK, so I have a crash that happens randomly and I have asked the people
> here
> to send me their logs when they crash, I have narrowed it down to one
> operation and I don't understand why it's happening. I just thought I
> could
> wrap it in a try/catch and handle it from there.

Using any kind of catch in that case is the last thing you should do because
in that way you will just masquerade problem for a while, but it will bite
you badly later on. There was a plenty of discussions on why catching SEH
exceptions in is a bad thing (unless you really know what you are doing).
What you should do is to let you process crash and collect minidump (make
sure you generate PDB files during build).
And a bit of advise: do not use catch(...) either it is broken on WIN32
platform and it is hard to fix it.
See
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&threadm=eD%24n1NTgDHA.632%40tk2msftngp13.phx.gbl&rnum=4&prev=/groups%3Fhl%3Den%26lr%3D%26ie%3DUTF-8%26q%3Dcatch(...)%2BVladimir

A good source of information on how to work with dump files is WinDbg help
files:
http://www.microsoft.com/whdc/devtools/debugging/installx86.mspx

Vladimir.

SteveK
Tue Sep 28 13:20:18 CDT 2004

Doug, this is all usefull and sound advice, thank you.
I plan to give some time to finding the bug that is passing junk to
GetRecordTypeId as that is indeed the problem. The code is such a rat's
nest of band-aides and hacks that I really don't intend to spend too much
time "fixing it correctly"

It would be valuable as a learning experience to hunt it down and that is my
motivation for doing that, but for the moment, I just want to handle this
junk pointer problem as gracefully as possible.

I will read about structured exceptions, hopefully there will be some code
samples in the link that will show me a trick to gracefully step over(or
around) this stinking pile of trash! ;)

Thanks again!


"Doug Harrison [MVP]" <dsh@mvps.org> wrote in message
news:q49jl01fqnlip9v7vhi6irdvvv2igk1866@4ax.com...
> SteveK wrote:
>
> >There is a tool that we use here at work. The guy that wrote it has since
> >quit and I was asked if I would like to take it over (I'm learning to
> >program and the co. supports that) with that said, the code is really
messy
> >and rather than fix ir "correct" i have started to design a new version
from
> >the ground up, I am still responsible for keeping the old one running
until
> >I roll out the new one.
> >
> >OK, so I have a crash that happens randomly and I have asked the people
here
> >to send me their logs when they crash, I have narrowed it down to one
> >operation and I don't understand why it's happening. I just thought I
could
> >wrap it in a try/catch and handle it from there.
> >
> >Here is the code:
> >inline byte GetRecordTypeId(void* pV)
> >{
> > if(pV)
> > {
> > try
> > {
> > return *(byte*)pV;
> > }
> > catch(char* e)
> > {
> > theLog.WriteError(e, __FILE__, __LINE__);
> > }
> > }
> > return kRecordTypeUndefined;
> >}
> >
> >
> >However, this appears to do nothing. It still crashes just the same as
if
> >there was no try block. From what I understand, the above code should
have
> >worked fine. What am I missing?
>
> Exceptions don't work like that. If dereferencing pV crashes, it's
> generating a Windows structured exception, not a C++ exception, and
> certainly not a char*. People sometimes (usually wrongly) want to catch
> structured exceptions in catch(...), and for more on that, see:
>
> http://members.cox.net/doug_web/eh.htm
>
> Note also that even if you were to catch the exception, the __FILE__ and
> __LINE__ usage probably wouldn't help you much, because they reflect the
> file and line in which they appear. Your problem appears to be one of
> garbage input to GetRecordTypeId, and what you have tried won't tell you
> where the call came from. IOW, the above is not giving you any new
> information, as you already seem to know it's crashing there. In the
> debugger, you could easily determine who called GetRecordTypeId by looking
> in the call stack window. If you don't know how to use the minidump Jochen
> mentioned (I confess I don't know how to do this - any tips, Jochen?), and
> you can't otherwise debug the crash, you can always search your source
code
> for GetRecordTypeId. If that function is called from just a few places,
you
> might be able to spot the problem without too much trouble. In addition,
you
> can always instrument your code with OutputDebugString calls and view the
> output using Sysinternals DbgView:
>
> http://www.sysinternals.com/ntw2k/freeware/debugview.shtml
>
> --
> Doug Harrison
> Microsoft MVP - Visual C++

Jeff
Tue Sep 28 13:23:37 CDT 2004


"Steve McLellan" <sjm.NOSPAM AT fixerlabs DOT com> wrote in message
news:ecF3zOYpEHA.1712@tk2msftngp13.phx.gbl...
> >
> > Here is the code:
> > inline byte GetRecordTypeId(void* pV)
> > {
> > if(pV)
> > {
> > try
> > {
> > return *(byte*)pV;
> > }
> > catch(char* e)
> > {
> > theLog.WriteError(e, __FILE__, __LINE__);
> > }
> > }
> > return kRecordTypeUndefined;
> > }
> >
> >
> catch(char* e) isn't a great way of catching the error, as you've found.
If
> you just want to try to trap everything, use catch(...)

You may need to ensure that the appropriate exception handling flags are in
place. I can't remember if an access violation is a hardware exception.
Since the only thing happening here is a dereference, using catch(...)
should be ok. Do a google on catch(...) to see the problems with it's usage.

> Your code is probably just bugging out when the cast fails.

That's doubtful, assuming a byte is typedef'd as one of the char types, and
that the actual exception is an access violation. I'd say the pointer is
being trashed upstream somewhere, since a check for NULL is not failing.

> This isn't great C++ programming :-)

Agreed. You should only need to be doing this sort of casting in very
limited situations.

> Apart from that, if you can't redesign the code (and it sounds like
> rebuilding is a good idea) you're better using a dynamic_cast - Google for
> it. Make sure you enable RunTime Type Info in the project properties (I
> think it's Compiler/Advanced or possibly Compiler/Language), and try (this
> is uncompiled, BTW):
>
> byte *b = dynamic_cast<byte *>( pV );

This is valid only for user defined classes and results in:

error C2680: 'char *' : invalid target type for dynamic_cast
target type must be a pointer or reference to a defined class

Did you mean reinterpret_cast?

> if ( b != NULL )

The OP already does this with the original code "if( pV )".



Jeff

Steve
Tue Sep 28 13:24:02 CDT 2004

Ah, yeah, it does - sorry. I'm an idiot :-) dynamic_cast works on objects
with v-tables, but not, as you right say, on bytes.

Short answer: catch(...) should trap the exception.
Long answer...
In that case, the error you're getting suggests you're deferencing something
which isn't there (or isn't valid). You checking against NULL as in the
following...

if ( pV != NULL )
{
byte *b = (byte *)(pV);
return *b;
}
...

The cast will always succeed, but at least you won't dereference a null
pointer. However, if pV isn't null, you then have no way of checking what pV
is (or should be). Using catch(...) will pick it up if pV is NULL. It sounds
like you have other problems in addition tho, if your function is receiving
bad pointers.

Steve

"SteveK" <asasd@asdfasdfsd.com> wrote in message
news:eljB7WYpEHA.1816@TK2MSFTNGP09.phx.gbl...
> Hi Steve,
>
> Thanks for the quick response. I quickly tried to implement your sample
and
> received compiler errors:
> "error C2680: 'byte *' : invalid target type for dynamic_cast target type
> must be a pointer or reference to a defined class file.cpp"
>
> "error C2680: 'byte *' : invalid target type for dynamic_cast"
>
>
> So I will start the google process to see what I can find, but in the
> meantime.. does this make any sense?
>
>
>
>
>
>
> "Steve McLellan" <sjm.NOSPAM AT fixerlabs DOT com> wrote in message
> news:ecF3zOYpEHA.1712@tk2msftngp13.phx.gbl...
> > >
> > > Here is the code:
> > > inline byte GetRecordTypeId(void* pV)
> > > {
> > > if(pV)
> > > {
> > > try
> > > {
> > > return *(byte*)pV;
> > > }
> > > catch(char* e)
> > > {
> > > theLog.WriteError(e, __FILE__, __LINE__);
> > > }
> > > }
> > > return kRecordTypeUndefined;
> > > }
> > >
> > >
> > catch(char* e) isn't a great way of catching the error, as you've found.
> If
> > you just want to try to trap everything, use catch(...) Your code is
> > probably just bugging out when the cast fails. This isn't great C++
> > programming :-)
> > Apart from that, if you can't redesign the code (and it sounds like
> > rebuilding is a good idea) you're better using a dynamic_cast - Google
for
> > it. Make sure you enable RunTime Type Info in the project properties (I
> > think it's Compiler/Advanced or possibly Compiler/Language), and try
(this
> > is uncompiled, BTW):
> >
> > byte *b = dynamic_cast<byte *>( pV );
> > if ( b != NULL )
> > {
> > return *b;
> > }
> > else
> > {
> > return kRecordTypeUndefined;
> > }
> >
> >
>
>

Steve
Tue Sep 28 13:25:11 CDT 2004

>
>
> No. See my other message for the why. :)

I stand corrected... :-)

>
> >
> You can't use the dynamic_cast operator for that.
>

And I _did_ know that one - normal lame excuses about late nights etc :-)

Igor
Tue Sep 28 16:16:32 CDT 2004

"SteveK" <asasd@asdfasdfsd.com> wrote in message
news:umHdaSYpEHA.3300@TK2MSFTNGP12.phx.gbl
> right you are, sorry about that!
>
> "Unhandled exception at 0x00466bf7 in Moves2.exe: 0xC0000005: Access
> violation reading location 0x0000029a."

Somebody passed a bogus pointer to you. The bug is in the calling code,
not in this function. try/catch won't help any - drop them.
--
With best wishes,
Igor Tandetnik

"On two occasions, I have been asked [by members of Parliament], 'Pray,
Mr. Babbage, if you put into the machine wrong figures, will the right
answers come out?' I am not able to rightly apprehend the kind of
confusion of ideas that could provoke such a question." -- Charles
Babbage

Jochen
Wed Sep 29 01:26:42 CDT 2004

SteveK wrote:

> right you are, sorry about that!
>
> "Unhandled exception at 0x00466bf7 in Moves2.exe: 0xC0000005: Access
> violation reading location 0x0000029a."

Please find out what the function does and who is calling it!

Then you can find the source of the problem...

If you use try/catch you are only digging in the dust... and your program
might not work correctly!

By the way: You never mentioned that the exception occurs in the code you
had shown us!

--
Greetings
Jochen

My blog about Win32 and .NET
http://blog.kalmbachnet.de/

ak
Wed Sep 29 07:18:22 CDT 2004

On Tue, 28 Sep 2004 10:38:50 -0700, "SteveK" <asasd@asdfasdfsd.com> wrote:

>There is a tool that we use here at work. The guy that wrote it has since
>quit and I was asked if I would like to take it over (I'm learning to
>program and the co. supports that) with that said, the code is really messy
>and rather than fix ir "correct" i have started to design a new version from
>the ground up, I am still responsible for keeping the old one running until
>I roll out the new one.
>
>OK, so I have a crash that happens randomly and I have asked the people here
>to send me their logs when they crash, I have narrowed it down to one
>operation and I don't understand why it's happening. I just thought I could
>wrap it in a try/catch and handle it from there.
>
>Here is the code:
>inline byte GetRecordTypeId(void* pV)
>{
> if(pV)
> {
> try
> {
> return *(byte*)pV;
> }
> catch(char* e)
> {
> theLog.WriteError(e, __FILE__, __LINE__);
> }
> }
> return kRecordTypeUndefined;
>}
>
>
>However, this appears to do nothing. It still crashes just the same as if
>there was no try block. From what I understand, the above code should have
>worked fine. What am I missing?
>
>For testing purpose I forced a crash in the code by setting the void* to an
>int (pV = (void*)someInt;)
>
>Any help or suggestions would be great.
>
>Thanks!
>

try adding a

catch(...)
{
}

instead of catch(char *)
--
You are only young once, but you can stay immature indefinitely.

SteveK
Thu Sep 30 11:17:16 CDT 2004

yes, the pointer was retrieved from the HTREEITEM that the user selected.
I'm pretty sure that what has happened is that hte underlying data has been
deleted or modifed and the tree item pointers aren't updated, hence the
junk. This should be a FUN one to find ;)

Thanks again all for the help!!


"Igor Tandetnik" <itandetnik@mvps.org> wrote in message
news:uExkrBapEHA.1712@tk2msftngp13.phx.gbl...
> "SteveK" <asasd@asdfasdfsd.com> wrote in message
> news:umHdaSYpEHA.3300@TK2MSFTNGP12.phx.gbl
> > right you are, sorry about that!
> >
> > "Unhandled exception at 0x00466bf7 in Moves2.exe: 0xC0000005: Access
> > violation reading location 0x0000029a."
>
> Somebody passed a bogus pointer to you. The bug is in the calling code,
> not in this function. try/catch won't help any - drop them.
> --
> With best wishes,
> Igor Tandetnik
>
> "On two occasions, I have been asked [by members of Parliament], 'Pray,
> Mr. Babbage, if you put into the machine wrong figures, will the right
> answers come out?' I am not able to rightly apprehend the kind of
> confusion of ideas that could provoke such a question." -- Charles
> Babbage
>
>