Access Violation Problem

M

Mike Copeland

I have the following code that compiles but produces a "Access
violation reading location..." runtime error when executed. I tried
casting the first statement to (char*), but that produced the same
fault.
I suppose the problem lies with the use of the "const char" in the
parameter declaration, but I've been trying to follow the examples I see
here that always seem to declare incoming parameters as "const"). 8<{{
Other than that, I don't know how to deal with this problem. (I'm
using VC 2010 Express here.) Please advise. TIA

char* TTB(const char *str) // Truncate Trailing Blanks
{
string wStr = str; // faults at this statement
wStr = trim_right(wStr);
return (char*)wStr.c_str();
} // TTB
 
I

Ian Collins

I have the following code that compiles but produces a "Access
violation reading location..." runtime error when executed. I tried
casting the first statement to (char*), but that produced the same
fault.
I suppose the problem lies with the use of the "const char" in the
parameter declaration, but I've been trying to follow the examples I see
here that always seem to declare incoming parameters as "const"). 8<{{
Other than that, I don't know how to deal with this problem. (I'm
using VC 2010 Express here.) Please advise. TIA

char* TTB(const char *str) // Truncate Trailing Blanks

Why don't you use a more expressive name?
{
string wStr = str; // faults at this statement

What is a "string", a std::string or something else? There's nothing
wrong in initialising a std::string from a const char*. What does the
name "wStr" mean?
wStr = trim_right(wStr);
return (char*)wStr.c_str();

Assuming wStr is a std::string, this will end in tears. You are
returning the address of some local (to the function) data.
 
A

Alain Ketterlin

I have the following code that compiles but produces a "Access
violation reading location..." runtime error when executed. I tried
casting the first statement to (char*), but that produced the same
fault.
I suppose the problem lies with the use of the "const char" in the
parameter declaration, but I've been trying to follow the examples I see
here that always seem to declare incoming parameters as "const"). 8<{{
Other than that, I don't know how to deal with this problem. (I'm
using VC 2010 Express here.) Please advise. TIA

char* TTB(const char *str) // Truncate Trailing Blanks
{
string wStr = str; // faults at this statement
wStr = trim_right(wStr);
return (char*)wStr.c_str();
} // TTB

I can't see why the first statement should be wrong (without knowing how
you call TTB [*]), but I'm sure the last one is wrong: you're returning
a pointer to data that is deallocated before the actual return. You
can't use the return value of TTB.

-- Alain.

P/S: maybe you have another function on the same model removing leading
blanks, the result of which you pass to TTB...
 
B

Balog Pal

Mike Copeland said:
I have the following code that compiles but produces a "Access
violation reading location..." runtime error when executed.
char* TTB(const char *str) // Truncate Trailing Blanks
{
string wStr = str; // faults at this statement
wStr = trim_right(wStr);
return (char*)wStr.c_str();
} // TTB

The pointer you get from wStr.c_str() expression becomes invalid as soon as
you return from the function. So if you use it for anything it is undefined
behavior -- access violation is fair game.

Also c_str() returns const char * that you're not supposed to unconstify.

Try to play by the rules if you want a program that actually works.
 
T

Tobias Müller

Paavo Helde said:
(e-mail address removed) (Mike Copeland) wrote in


This function is invalid as noted by others, and also totally useless;
delete it and replace all calls to it with calls to trim_right(). Update
the calling code to receive a std::string instead of char*.

In the general case I find it more convenient to use "const char*" for
parameters instead of "const std::string&" because the function is more
universally usable like that. "const std::string&" -> "const char*" is
easier and faster than the other way. You can also use other string
classes, since probably all of them have "operator const char*()" defined.

My rules are:
- for parameters, use the most general/abstract types
- for return values and local variables, use the most specific/concrete
types

Tobi
 
T

Tobias Müller

Paavo Helde said:
This way you will lose the length information and are doomed to recalculate it;

Many algorithms don't need it or are traversing the entire string anyways.

Also recalculation is cheaper than creating a temporary std::string object
in case of const char*.
you will also lose the ability to process strings with embedded zeroes.

Never needed that. If you do, you cannot use any third party C libraries,
and I suspect that many C++ libraries are not safe too.
Also, calling code would be more complex as there is an automatic
conversion from const char* to const std::string&, but not vice versa.

But the conversion exists and is cheap.
If you want to support different string types, then templates would be the
right approach.

I could do that, but then I would need some kind of string class traits
first. This would probably be difficult for const char*.

Anyway, it's probably a matter of taste and depends on the frequency of
std::string and const char* in the rest of the code.

Tobi
 
M

Mike Copeland

This function is invalid as noted by others, and also totally useless;
delete it and replace all calls to it with calls to trim_right(). Update
the calling code to receive a std::string instead of char*.

Oh, and if indeed you get the access violation at the line you show, then
most probably this is because you are passing in a NULL pointer. Don't do
that, it would probably not work with trim_right() either (just guessing as
trim_right() is not shown).
The root of my problem was not the routine itself, but it was the way
I was using it. I was invoking it this way:

strcpy(EDate, TTB(tpCopy(s2, 1, 20)));

where "s2" was a C-style string and myCopy wasn't returning a "const
char*" to be used by TTB.
I have "fixed" the problem so that it all works, but I'm concerned
that "myCopy" is potentially faukty. Specifically, I wonder about the
many "malloc" calls by repeated calls to it (memory resource and/or
fragmentation), as well as the risk of returning a pointer that could be
removed before the called "gets" the data.
The code seems to work now, but am I invoking undefined behavior or
something that could crash my programs in the future? If so, how do I
remedy this?

char* tpCopy(const char *str, int ii, int nn)// get substring (TP
version)
{
if(strlen(str) <= 0) return "";
char *temp;
if(NULL != (temp = (char*)malloc(strlen(str)+1)))
{
temp = _strdup(str);
temp[nn] = '\0';
return temp; // can't deallocate this memory!
}
else
{
return ""; // some error code required here...
}
} // tpCopy

BTW, I have hundreds of programs and many thousand lines of code.
Although I'd like to convert my C-style string code to basic_string, the
effort is substantial. My current task is to upgrade my VC6.0 coding to
use VC 2010 Express (as suggested by many here), and that's a daunting
effort by itself! Hopefully, I get to these other conversions later (if
I live long enough: I'm 72)... 8<}}
 
B

Balog Pal

Mike Copeland said:
BTW, I have hundreds of programs and many thousand lines of code.
Although I'd like to convert my C-style string code to basic_string, the
effort is substantial. My current task is to upgrade my VC6.0 coding to
use VC 2010 Express (as suggested by many here), and that's a daunting
effort by itself! Hopefully, I get to these other conversions later (if
I live long enough: I'm 72)... 8<}}

basic_string? Forget it.

CString is your friend in that case -- it has interface suitable for about
all tasks you have in the envirionment, and has implicit conversion toward
const char *, so you can start replacing the c-style stuff without uglifying
the user code.

(As a bonus you also dodge a ton of problems if decide to put some code into
DLL).
 
J

Jorgen Grahn

basic_string? Forget it.

"Forget it?" Why? It's the standard string type in C++.
CString is your friend in that case -- it has interface suitable for about
all tasks you have in the envirionment, and has implicit conversion toward
const char *, so you can start replacing the c-style stuff without uglifying
the user code.

(As a bonus you also dodge a ton of problems if decide to put some code into
DLL).

/Jorgen
 
D

Dombo

Op 10-Jun-12 13:25, Balog Pal schreef:
basic_string? Forget it.

CString is your friend in that case -- it has interface suitable for
about all tasks you have in the envirionment, and has implicit
conversion toward const char *, so you can start replacing the c-style
stuff without uglifying the user code.

(As a bonus you also dodge a ton of problems if decide to put some code
into DLL).

Mike is using the Express edition of VC 2010, which doesn't come with
MFC nor ATL libraries, hence out-of-the-box no CString with this edition.
 
J

Juha Nieminen

Paavo Helde said:
This is about the worst advice one could give, exactly because of the
implicit conversion to const char*. This means that if some function f()
outputs a string, the following code compiles without any warning or
error:

const char* s = f();

Now imagine that f() returned a char* pointer originally (e.g. to a
static buffer, or to a string literal), and was later upgraded to return
a CString. I think this is exactly what OP wants to do.

I'm not absolutely sure of this, but I have the impression that if he's
using C++.NET (which isn't a far-fetched assumption given that he is
developing a Visual C++ project), the string in question would be
gargabe-collected and hence the above would be safe. CString is probably
allocating the string using gcnew.

(OTOH I haven't checked if C++.NET only garbage-collects class instances,
or all allocated memory blocks.)
 
D

Dombo

Op 11-Jun-12 11:23, Juha Nieminen schreef:
I'm not absolutely sure of this, but I have the impression that if he's
using C++.NET (which isn't a far-fetched assumption given that he is
developing a Visual C++ project), the string in question would be
gargabe-collected and hence the above would be safe. CString is probably
allocating the string using gcnew.

I doubt that (can't check it because I don't have the Professional
Edition of VC2010). Am sure this isn't true for the version of MFC and
ATL that came with VC2005, and I seriously doubt Microsoft would change
this as it would imply you couldn't use MFC or ATL with unmanaged
applications, which would kinda defeat the purpose of including these
libraries in the first place. Note that VC2010 still can produce native
executables which do not require the .NET framework.

As far as the .NET garbage collection thingy is concerned; that doesn't
do anything for raw C++ pointers, the onus is still on the programmer to
take care of the lifetime. I have recently looked in to this for a
project that required C# code to communicate with unmanaged C++ code.
C++/CLI is a schizophrenic language. There is C++ part with C++
semantics and the CLI part with the semantics typical for .NET
languages, including garbage collection. Those two parts are quite
separated and don't mix easily. If you want .NET garbage collection, you
can only use a 'ref class', which defines a .NET class. A 'ref class'
cannot derive from C++ classes, or have C++ class instances as members,
must be instantiated with gcnew and instances must referred by handle
(e.g. my_type^). C++ classes cannot derive from .NET classes or directly
have references to instances of .NET classes (at least not directly at
the language level). Compiling standard C++ code doesn't get you any of
the .NET features. Because of this and the reasons stated above I don't
expect that the CString implementation would be safe to use in the
example posted above.

Given that the OP stated that he is using the Express Edition of VC2010,
using CString isn't an option, using basic_string is the only choice
that is supported out-of-the-box (unless he gets a copy of a suitable
version of the MFC/ATL library and manages to use it with the Express
Edition).
 
B

Balog Pal

Paavo Helde said:
This is about the worst advice one could give, exactly because of the
implicit conversion to const char*. This means that if some function f()
outputs a string, the following code compiles without any warning or
error:

const char* s = f();

And we arrived to gremlin-land. Yeah, that means that. So then what?

In C++ we have too many opportunities for dongling objects and references.
So any actually working system deals with that on reviews. Generally you're
not supposed to use raw pointers. And if any pointer or ref is stored, there
must be a proof for correct lifetime issues. Conversions, implicit or
explicit, adds quite nothing to that case.
Now imagine that f() returned a char* pointer originally (e.g. to a
static buffer, or to a string literal), and was later upgraded to return
a CString. I think this is exactly what OP wants to do.

You can hardly sell that conversion as sensible on a review.

If a static lifetime char* was returned, then you must return CString with a
similar lifetime, so the client code will just work fine. If you change
lifetime issues, like returning a temporary in stead of a stable thing, you
must revisit all usege places and adjust them. Capturing the temporary
correctly.
I am sure you and me could foresee the dangers and avoid the problems,
but I am not so sure at all about OP.

Whoever can't design the flow of data in a system is doomed to failure
regardless of tools used. Didn't the 'new age' languages like java,
promising Canaan with garbage collection enough proof? Making all the same
problems actually escalated?
std::basic_string works as fine in dll-s as do std::vector and std::map,
not sure what problems you have in mind.

WOW, did you read MSDN recently or ever? Or tried to use them? You get
violent warning right ahead about the templates not being exported by
nature. Then will have an uphill battle. I didn't even mention what you get
for mixing ITERATOR_DEBUG_LEVEL, where you may get a linker warning at
least, but may just silently have ODR violation and the wildest crashes
imaginable...
 
B

Balog Pal

Jorgen Grahn said:
"Forget it?" Why? It's the standard string type in C++.

"standard" is not a synonym for "good". I believe there's a wide consensus
that std::string is an abomination with the interface it has. And in thoese
100+ members it still can't deal with the most basic use cases.

From the interface you can hardly tell what it was designed for, mostly
resembles a half-done vector. tuned for iterator usage, that is hardly done
ever. Actual string-like operations NOT really in mind.

Not to mention accidents like it was supposedly designed to use COW
strategy, but due to broken accessor interface that had to be forbidden
implementation for thread-unsafety reasons. Annonuced like a year after the
standard published.

It's quite a shame, by '98 we had a ton of actually useful, used, GOOD
string classes, possibly with a decade+ of practical use. The
standardisation would suppose to not invent speculative stuff, rather pick
what is good from actual use, and put it in formal terms. I did not happen
for std::string (and many other parts of the standard lib.)
 
J

Jorgen Grahn

"standard" is not a synonym for "good". I believe there's a wide consensus
that std::string is an abomination with the interface it has.

That was what I thought you meant. There is *not* such a consensus --
although I'm aware that some (a lot of?) people are unhappy with it.

[snip]

/Jorgen
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,756
Messages
2,569,534
Members
45,007
Latest member
OrderFitnessKetoCapsules

Latest Threads

Top