• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 858
  • Last Modified:

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

0
Cyber-Dragon
Asked:
Cyber-Dragon
  • 8
  • 5
  • 2
2 Solutions
 
evilrixSenior Software Engineer (Avast)Commented:
       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/
0
 
evilrixSenior Software Engineer (Avast)Commented:
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.
0
 
numberkruncherCommented:
I recommend that you use the std::string class for this. If you do choose to proceed with the C-style approach then your static buffer must be within the scope of your call because otherwise you may encounter data corruption where the stack is reused.

Or you can dynamically allocate your memory within the "NewFile" function, but the memory MUST be cleaned up afterwards once you have finished with the combined string. Something like the following should do the trick.
char* RandomState()
{
        char* Billy = "Billy";
 
        return Billy;
}
 
char* RandomCity()
{
        char* Billy = "Billy"
 
        return Billy;
}
 
char* NewFile()
{
        char* state = RandomState();
        char* city = RandomCity();
 
        char* combine = new char[strlen(state) + strlen(city) + 1];
        *combine = '\0';
        strcat(combine,state);
        strcat(combine, city);
 
        char* newFile = combine;
 
        return newFile;
}
 
void DoSomething()
{
        char* foo = NewFile();
 
        // Your logic
 
        // Clean up foo.
        delete[] foo;
}

Open in new window

0
Independent Software Vendors: 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!

 
Cyber-DragonAuthor Commented:
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

0
 
evilrixSenior Software Engineer (Avast)Commented:
>> 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!
0
 
numberkruncherCommented:
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

0
 
evilrixSenior Software Engineer (Avast)Commented:
>> That helped solve that issue but now its not compatible with my function that required the char const*
Just use a string, see below for how simple this is when using safe C++ types.
#include <string>
 
char const * RandomState()
{
        return "Billy";
}
 
char const * RandomCity()
{
        return "Goat";
}
 
std::string NewFile()
{
		return std::string(RandomState()) + RandomCity();
}
 
int main()
{
	std::string const & sNewFile = NewFile();
	char const * szNewFile = sNewFile.c_str();
}

Open in new window

0
 
Cyber-DragonAuthor Commented:
numberkruncher's solution worked. Thank you.
0
 
numberkruncherCommented:
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.
0
 
evilrixSenior Software Engineer (Avast)Commented:
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.
0
 
evilrixSenior Software Engineer (Avast)Commented:
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

0
 
numberkruncherCommented:
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

0
 
numberkruncherCommented:
So the output to that example is:

Hello World!DEMO TEXTPress any key to continue . . .
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> 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.
0
 
evilrixSenior Software Engineer (Avast)Commented:
>> different calling conventions manipulate the stack different, some preserve it
http://en.wikipedia.org/wiki/X86_calling_conventions
http://www.codeproject.com/KB/cpp/calling_conventions_demystified.aspx
0

Featured Post

Technology Partners: 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!

  • 8
  • 5
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now