Link to home
Start Free TrialLog in
Avatar of Cyber-Dragon
Cyber-Dragon

asked on

C++ Combing Two Char*

Hi,

I'm trying to generate two items like City and State in two separate functions and then combine them into one const char* to be used in another function.

I'm getting a memory access violation for some reason and I'm not sure what to do.
char* RandomState()
{
	char* Billy = "Billy";
 
	return Billy;
}
 
char* RandomCity()
{
	char* Billy = "Billy"
 
	return Billy;
}
 
char* NewFile()
{
	char* combine;
	strcat(combine,RandomState());
	strcat(combine,RandomCity());
 
	char* newFile = combine;
 
	return newFile;
}

Open in new window

Avatar of evilrix
evilrix
Flag of United Kingdom of Great Britain and Northern Ireland image

       char* combine;
        strcat(combine,RandomState());
        strcat(combine,RandomCity());

combine is just a pointer, it doesn't point to anything so you are trying to strcat to a random aread of memory. Try changing combine to be an array (make sure it's big enough!)...

        char combine [500] = {0}; // Make sure you initialise it to empty first!
        strcat(combine,RandomState());
        strcat(combine,RandomCity());

http://www.cplusplus.com/doc/tutorial/arrays/
http://www.cplusplus.com/doc/tutorial/ntcs/
http://www.cplusplus.com/doc/tutorial/pointers/

Better still (and more C++ like) just use a std::string

        std::string combine;
        combine += RandomState();
        combine += RandomCity();

If you need to get a char const * from it just use the c_str() function


char const * p = combine.str(); // p is only valid for as long as combine is in scope and has NOT been modified in any way.

http://www.cplusplus.com/reference/string/string/
BTW: The following is NOT valid C++

char* Billy = "Billy";

The use of a none const pointer to char to point to a string literal is deprecated in C++. Most (up-to-date and standards compliant) compilers will warn you of this. Use the const version, thus...

char const * Billy = "Billy";

Of course, you'll also need to change your function return types.

----------------------------------------------------------------------------------------------------------------------------------------------------------------------
ISO/IEC 14882:2003(E) C.1.1 Clause 2: lexical conventions Annex C Compatibility

Change: String literals made const

The type of a string literal is changed from array of char to array of const char. The type of a wide string literal is changed from array of wchar_t to array of const wchar_t.

Rationale: This avoids calling an inappropriate overloaded function, which might expect to be able to modify its argument.

Effect on original feature: Change to semantics of well-defined feature.

Difficulty of converting: Simple syntactic transformation, because string literals can be converted to
   char*; (4.2). The most common cases are handled by a new but deprecated standard conversion:
   char* p = "abc"; // valid in C, deprecated in C + +
   char* q = expr ? "abc" : "de"; // valid in C, invalid in C + +

How widely used: Programs that have a legitimate reason to treat string literals as pointers to potentially modifiable memory are probably rare.
ASKER CERTIFIED SOLUTION
Avatar of numberkruncher
numberkruncher
Flag of United Kingdom of Great Britain and Northern Ireland image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of Cyber-Dragon
Cyber-Dragon

ASKER

That helped solve that issue but now its not compatible with my function that required the char const*



char const* NewPath = NewFile();
 
//This file is not opening
std::ofstream out(NewPath,ofstream::binary); // open target file

Open in new window

>> I recommend that you use the std::string class for this.
I'm sure I already did that 20 minutes beforehand, no? {http:#24169593}

>> you may encounter data corruption where the stack is reused.
If the local scoped array has gone out of scope then any pointers to it will be dangling but there will be no corruption!
There shouldn't be a problem casting from char* to const char*.

Perhaps the error is with your delete code?
char *NewPath = NewFile();
 
//This file is not opening
std::ofstream out(NewPath,ofstream::binary); // open target file
 
delete[] NewPath;

Open in new window

SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
numberkruncher's solution worked. Thank you.
evilrix@

Your right, there should not be any data corruption.

I only re-iterated std::string because that is probably the best way to go, but I just wanted to mention dynamic strings. Perhaps I should have worded that differently.
BTW: The use of new [] and delete [] (or their scalar counter-parts) without RAII (eg a smart pointer) is not exception safe and should be avoided. Unfortunately, C++ doesn't provide a smart pointer for array new/delete so there is no safe way to do this would using a string or a vector.
If you are just using string literals then there is no need for any memory allocation at all since you can combine them at compile time.
#define RandomState "Billy"
#define RandomCity "Goat"
 
char const * NewFile()
{
		return RandomState RandomCity;
}
 
int main()
{
	char const * szNewFile = NewFile();
}

Open in new window

evilrix@

I have just double checked, I am right, static arrays are constructed on the stack. Once the called function has exited, the stack frame pointer moves upwards, and then the calling function overwrites the originally statically created array:
// StackTest.cpp : Defines the entry point for the console application.
//
 
#include "stdafx.h"
#include <string.h>
 
char* test();
char* foo();
 
int _tmain(int argc, _TCHAR* argv[])
{
	char *result = test();
 
	foo();
 
	// result is likely to be corrupted with "DEMO TEXT".
	printf(result);
 
	return 0;
}
 
char* test()
{
	char buffer[255] = {0};
	strcat(buffer, "Hello");
	strcat(buffer, " ");
	strcat(buffer, "World!");
 
	// SAFE
	printf(buffer);
 
	return buffer;
}
 
char* foo()
{
	char buffer[255] = {0};
	strcat(buffer, "DEMO TEXT");
	return buffer;
}

Open in new window

So the output to that example is:

Hello World!DEMO TEXTPress any key to continue . . .
>> I am right, static arrays are constructed on the stack
I know, I didn't say they weren't :)

The point is that once the local array goes out of scope (and is popped from the stack) it is no longer valid and any pointers to it are dangling. What happens to the stack-frame after that is not relevent. In other-words you are seeing a side-effect the fact is the array no longer exists and is, in fact no longer valid memory. This being the case the definition you use of it getting corrupt isn't true because it's no longer valid memory therefore what happens to it after that is (a) undefined [for example, different calling conventions manipulate the stack different, some preserve it] and (b) irrelevant because the memory is not longer valid anyway.

Does my assertion now make sense?

You'll see that this is why I kept promoting the use of std::string, because it can be returned by value, unlike an array. You can, of course, wrap the array in a struct and return that by value but it's not a very efficient solution to a simple problem.