Why does my C-String display weird symbols?

  • (2 Pages)
  • +
  • 1
  • 2

16 Replies - 1463 Views - Last Post: 29 June 2017 - 04:49 PM Rate Topic: -----

#1 The Shroom  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 21
  • Joined: 06-April 16

Why does my C-String display weird symbols?

Posted 28 June 2017 - 02:21 PM

I'm having a problem where my char* variable turns into weird symbols both when printing it to the screen and when checking it's memory. All I know as of right now is that the C-String is null terminated since the last element of the char* + 1 is equal to \0.
I'm not sure, but I think whatever my char* is pointing to, is running out of scope.

	void Shader::LoadShader(const char* shaderName, const char* path)
	{
		std::ifstream file;
		file.open(path);
		if (file.good() == false)
		{
			Logging::Log("Failed to open shader file : " + std::string(path));
			m_shaderSource = "NULL";
			m_shaderPath = "NULL";
			m_shaderName = "NULL";
			return;
		}

		std::string m_shaderSourceStr{ std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
		m_shaderSource = (char*)m_shaderSourceStr.c_str();
		m_shaderSource += '\0'; // Need to null-terminate the string, since it doesn't do that automatically.
		m_shaderName = (char*)shaderName;
		m_shaderPath = (char*)path;

		std::cout << "m_shaderSource: " << &m_shaderSource[0] << std::endl;
		std::cout << "m_shaderSourceStr: " << &m_shaderSourceStr[0] << std::endl;
		std::cout << "m_shaderSourceStr.c_str(): " << &m_shaderSourceStr.c_str()[0] << std::endl;

		file.close();
	}



So, the variable which for some reason is running out of scope after this function is m_shaderSource.
I think it's because of the std::string m_shaderSourceStr which I set the m_shaderSource to, but I don't really understand why.

I'm setting m_shaderSource to m_shaderSourceStr.str(), and I assume because the str() function returns a char pointer, it sets the m_shaderSource to be the first element in the str() pointer, and when m_shaderSourceStr runs out of scope, the str() fucntion does to, and since m_shaderSource points to that, it also runs out of scope.

At least that's what I think is happening but I'm not sure.

If I am correct and that is what's happening, I would very much like to know how I would get the contents of the std::ifstream file, without it running out of scope? Maybe I can somehow make a copy of str() instead of actually using it's pointer?

Is This A Good Question/Topic? 0
  • +

Replies To: Why does my C-String display weird symbols?

#2 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 5725
  • View blog
  • Posts: 19,340
  • Joined: 05-May 12

Re: Why does my C-String display weird symbols?

Posted 28 June 2017 - 04:49 PM

There are so many issues with your code. Looks like a basic lack of knowledge of C and C++ strings. Please don't take this personally, but should you be playing with shaders when you can't even play with strings correctly?

There is no need to null terminate the return value of c_str(). It is guaranteed to be null terminated.
Was This Post Helpful? 1
  • +
  • -

#3 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 5725
  • View blog
  • Posts: 19,340
  • Joined: 05-May 12

Re: Why does my C-String display weird symbols?

Posted 28 June 2017 - 05:05 PM

Yes, m_shaderSourceStr goes out of scope and so the C++ string deallocates the memory it allocated. Since you setup m_shaderSource to point to the memory being use by the C++ string, when the string is deallocated, you are now pointing at a chunk of memory that the system is free to do whatever it pleases, including putting junk data.

Part of the issue here is that you are using Hungarian naming but apparently don't understand what it means. The m_ prefix is meant to tell the person reading the code that the variable is a "member" variable. Member variables have class scope. Unfortunately, in your code above, you declared m_shaderSourceStr as a local variable. Local variables as you've discovered has local scope.
Was This Post Helpful? 1
  • +
  • -

#4 The Shroom  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 21
  • Joined: 06-April 16

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 06:00 AM

Yeah, I have not played with the functions of strings in general very much, but I'll go ahead and watch some tutorials about it.

And about the m_ prefix, not sure why I added it to m_shaderSourceStr. I do know that they should be only added to member variables though :)/>/>/>

And I read somewhere that c_str() did not add a null-terminator, but I guess I should be checking my sources before trusting them.

This is how my code looks now, and it seems to be working properly : (however, there are probably better ways to go about doing this, will check that later but now I will be reading up on strings :P/>/>)

	void Shader::LoadShader(const char* shaderName, const char* path)
	{
		std::ifstream file;
		file.open(path);
		if (file.good() == false)
		{
			Logging::Log("Failed to open shader file : " + std::string(path));
			m_shaderSource = "NULL";
			m_shaderPath = "NULL";
			m_shaderName = "NULL";
			return;
		}

		std::string shaderSourceStr{ std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
		m_shaderSource = new char();
		strcpy(m_shaderSource, shaderSourceStr.data());
		m_shaderName = (char*)shaderName;
		m_shaderPath = (char*)path;

		std::cout << "m_shaderSource: " << &m_shaderSource[0] << std::endl;
		std::cout << "m_shaderSourceStr: " << &shaderSourceStr[0] << std::endl;
		std::cout << "m_shaderSourceStr.c_str(): " << &shaderSourceStr.c_str()[0] << std::endl;

		file.close();
	}



Thanks for the help! :)/>/>

This post has been edited by Skydiver: 29 June 2017 - 10:16 AM
Reason for edit:: Removed unnecessary quote. No need to quote the post above yours.

Was This Post Helpful? 0
  • +
  • -

#5 jimblumberg  Icon User is offline

  • member icon

Reputation: 5245
  • View blog
  • Posts: 16,297
  • Joined: 25-December 09

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 06:25 AM

Look at this snippet:
        m_shaderSource = "NULL";
        m_shaderPath = "NULL";
        m_shaderName = "NULL";

How were these variables defined (what type)? Are they std::strings or C-strings? Also why are you trying to place the string "NULL" into these strings?

Next:
   m_shaderSource = new char();
   strcpy(m_shaderSource, shaderSourceStr.data());

Do you realize that you're attempting to allocate a single character to m_shaderSource? And then you're trying to copy a C-string into this single character causing a buffer overflow error? Also you should not be using the data() member function with strcpy, use the c_str() member function instead.

Next:
   m_shaderName = (char*)shaderName;
   m_shaderPath = (char*)path;



What exactly are you trying to do here? Do you realize that you can't use the assignment operator= with C-strings? Also are you aware of the fact that both of the pointers to path and shaderName are local to the function and go out of scope when the function ends?


Why are you even playing with error prone C-strings when you seem to know about std::string. I would re-write this function to use std::string instead of C-strings and save yourself a log of grief, especially since you don't seem to understand how to use C-strings.

Jim

This post has been edited by jimblumberg: 29 June 2017 - 06:28 AM

Was This Post Helpful? 4
  • +
  • -

#6 The Shroom  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 21
  • Joined: 06-April 16

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 08:40 AM

The variables m_shaderSource, m_shaderPath and m_shaderName are all private c-string variables of the same class which the function is defined in. The reason I set it to "NULL" was to check if the file opened correctly (I printed the variables to the console).

The reason I allocated a single char to m_shaderSource was since you cannot use a uninitialized variable in strcpy, but I assume I should be using malloc instead?

And why can you not use the assignment operator on a c-string? (Can't seem to find much about it on google)

I now have this code :

	void Shader::LoadShader(const char* shaderName, const char* path)
	{
		std::ifstream file;
		file.open(path);
		if (file.good() == false)
		{
			Logging::Log("Failed to open shader file : " + std::string(path));
			m_shaderSource = "NULL";
			m_shaderPath = "NULL";
			m_shaderName = "NULL";
			return;
		}

		std::string shaderSourceStr{ std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
		m_shaderSource = (char*)malloc(shaderSourceStr.size());
		strcpy(m_shaderSource, shaderSourceStr.c_str());
		strcpy(m_shaderName, shaderName);
		strcpy(m_shaderPath, path);

		file.close();
	}


This post has been edited by Skydiver: 29 June 2017 - 10:16 AM
Reason for edit:: Removed unnecessary quote. No need to quote the post above yours.

Was This Post Helpful? 0
  • +
  • -

#7 #define  Icon User is offline

  • Duke of Err
  • member icon

Reputation: 1846
  • View blog
  • Posts: 6,619
  • Joined: 19-February 09

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 08:50 AM

Hi, a C type string is an array and you cannot assign an array to an array using the basic language syntax.

Character sequences
Was This Post Helpful? 3
  • +
  • -

#8 jimblumberg  Icon User is offline

  • member icon

Reputation: 5245
  • View blog
  • Posts: 16,297
  • Joined: 25-December 09

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 09:32 AM

By the way you didn't answer my biggest question and concern, namely:

Quote

Why are you even playing with error prone C-strings when you seem to know about std::string. I would re-write this function to use std::string instead of C-strings and save yourself a log of grief, especially since you don't seem to understand how to use C-strings.


And please show a small sample of your input file (inside code tags to preserve formatting).




Jim
Was This Post Helpful? 0
  • +
  • -

#9 The Shroom  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 21
  • Joined: 06-April 16

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 10:03 AM

Oh sorry, the reason I'm using c-strings is because I want to learn more about them, to prevent things like this from happening in the future. (I will switch to std::strings when I feel like I have a working c-string version though)

The input file is simply the words "Hello World" which I'm attempting to cout to the screen.

However everything seems to be working now, everything is printing to the console as it should, without any weird symbols.

Edit : Nope, it does not work, crashes on line 480 in iosfwd.h :

static size_t __CLRCALL_OR_CDECL length(_In_z_ const _Elem * const _First) _NOEXCEPT // strengthened
{	// find length of null-terminated string
	return (_CSTD strlen(_First)); // <-- Crashes here
}



It crashes with the error :
Exception thrown at 0x5242F1E0 (ucrtbased.dll) in RebornEngine.exe: 0xC0000005: Access violation reading location 0x00000000.



And in my code it crashes when trying to cout the shader's path :
std::cout << "Default shader path: " << defaultShader.GetPath() << std::endl;



The GetPath() function simply returns the m_shaderPath variable.

Here's my current LoadShader function :

	void Shader::LoadShader(const char* shaderName, const char* path)
	{
		std::ifstream file;
		file.open(path);
		if (file.good() == false)
		{
			Logging::Log("Failed to open shader file : " + std::string(path));
			m_shaderSource = "NULL";
			m_shaderPath = "NULL";
			m_shaderName = "NULL";
			return;
		}

		std::string shaderSourceStr{ std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
		m_shaderSource = (char*)malloc(shaderSourceStr.size());
		m_shaderName = (char*)malloc(strlen(shaderName));
		m_shaderPath = (char*)malloc(strlen(path));
		strcpy(m_shaderSource, shaderSourceStr.c_str());
		strcpy(m_shaderName, shaderName);
		strcpy(m_shaderPath, path);

		file.close();
	}


This post has been edited by Skydiver: 29 June 2017 - 10:19 AM
Reason for edit:: Removed unnecessary quote. No need to quote the post above yours.

Was This Post Helpful? 0
  • +
  • -

#10 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 5725
  • View blog
  • Posts: 19,340
  • Joined: 05-May 12

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 10:19 AM

The Shroom: There is no need to quote the post above yours. Just use the big Reply button or the Fast Reply area.
Was This Post Helpful? 0
  • +
  • -

#11 jimblumberg  Icon User is offline

  • member icon

Reputation: 5245
  • View blog
  • Posts: 16,297
  • Joined: 25-December 09

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 10:29 AM

Quote

Oh sorry, the reason I'm using c-strings is because I want to learn more about them, to prevent things like this from happening in the future.

What? You should prefer C++ strings and you should be learning more about them. Try to leave C-strings until later after you're more familiar with the language.

Quote

The GetPath() function simply returns the m_shaderPath variable.

Show your code, all of it.

The following is still wrong. You need to use strcpy() to copy a C-string not assignment.

			
			m_shaderSource = "NULL";
			m_shaderPath = "NULL";
			m_shaderName = "NULL";


Also:
Logging::Log("Failed to open shader file : " + std::string(path));

What type of variable does Log() expect as a parameter?

Next:

		m_shaderSource = (char*)malloc(shaderSourceStr.size());
		m_shaderName = (char*)malloc(strlen(shaderName));
		m_shaderPath = (char*)malloc(strlen(path));


You shouldn't be using malloc in a C++ program, you should be using new instead. Also how are those variables defined (show the class definition).

Jim
Was This Post Helpful? 0
  • +
  • -

#12 The Shroom  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 21
  • Joined: 06-April 16

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 11:25 AM

I won't be posting the full code since it's quite big, just the parts of it that actually "concerns" the problem I'm having.
(Might be wrong use of the word concern...)

Here's my main.cpp file :

#include <GLEW/GL/glew.h>

#include <iostream>

#include "core_engine/core.h"
#include "core_engine/helpers/logging.h"
#include "core_engine/input/input.h"
#include "core_engine/types/vectors.h"
#include "core_engine/graphics/shader_manager.h"

#undef main

using namespace RebirthEngine;

int main(int argc, char* argv[])
{
	
	Core engine = Core();
	if (engine.Initialize(argc, argv, "Rebirth Engine - OpenGL 3.3", 600, 400) == false)
	{
		Logging::GenerateCrashLog("Failed to initialize engine");
	}

	Shader defaultShader;
	defaultShader.LoadShader("Default", "C:/Users/Alex/Desktop/Projects/C++/Game Related/RebornEngine/TempData/Default.hlsl");
	std::cout << "Default shader path: " << defaultShader.GetPath() << std::endl;
	std::cout << "Default shader source: " << defaultShader.GetSource() << std::endl;
	std::cout << "Default shader name: " << defaultShader.GetName() << std::endl;

	Shader defaultShader2;
	defaultShader.LoadShader("Default2", "C:/Users/Alex/Desktop/Projects/C++/Game Related/RebornEngine/TempData/Default.hlsl");
	std::cout << "Default shader path: " << defaultShader2.GetPath() << std::endl;
	std::cout << "Default shader source: " << defaultShader2.GetSource() << std::endl;
	std::cout << "Default shader name: " << defaultShader2.GetName() << std::endl;

	while (engine.ShouldShutdown() == false)
	{
		engine.Update();

		engine.ProcessInput();

		engine.Render();
	}

	engine.Shutdown();
	return 0;
}



The Shader class files are these 2 :

Shader.h

#pragma once

namespace RebirthEngine { namespace Graphics {

	class Shader
	{

	public:

		Shader();
		~Shader();

		void LoadShader(const char* shaderName, const char* path);
		const char* GetName();
		const char* GetPath();
		const char* GetSource();

	private:

		char* m_shaderSource;
		char* m_shaderName;
		char* m_shaderPath;

	};

} }



Shader.cpp

#include "shader.h"
#include "../helpers/logging.h"
#include <fstream>
#include <vector>

namespace RebirthEngine { namespace Graphics {

	Shader::Shader()
	{

	}

	Shader::~Shader()
	{
		delete m_shaderName;
		delete m_shaderPath;
		delete m_shaderSource;
		m_shaderName = nullptr;
		m_shaderPath = nullptr;
		m_shaderSource = nullptr;
	}

	void Shader::LoadShader(const char* shaderName, const char* path)
	{
		std::ifstream file;
		file.open(path);
		if (file.good() == false)
		{
			Logging::Log("Failed to open shader file : " + std::string(path));
			strcpy(m_shaderSource, "NULL");
			strcpy(m_shaderName, "NULL");
			strcpy(m_shaderPath, "NULL");
			return;
		}

		std::string shaderSourceStr{ std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
		m_shaderSource = (char*)malloc(shaderSourceStr.size());
		m_shaderName = (char*)malloc(strlen(shaderName));
		m_shaderPath = (char*)malloc(strlen(path));
		strcpy(m_shaderSource, shaderSourceStr.c_str());
		strcpy(m_shaderName, shaderName);
		strcpy(m_shaderPath, path);

		file.close();
	}

	const char* Shader::GetName()
	{
		return m_shaderName;
	}

	const char* Shader::GetPath()
	{
		return m_shaderPath;
	}

	const char* Shader::GetSource()
	{
		return m_shaderSource;
	}

} }



The logging function has 2 overloads :

		static void Log(const std::string& msg);
		static void Log(const char* msg);


Was This Post Helpful? 0
  • +
  • -

#13 jimblumberg  Icon User is offline

  • member icon

Reputation: 5245
  • View blog
  • Posts: 16,297
  • Joined: 25-December 09

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 11:42 AM

Quote

I won't be posting the full code since it's quite big, just the parts of it that actually "concerns" the problem I'm having.

Then I'll probably only be able to offer my best guesses as to what is wrong, and I'm a really really bad guesser. You need to start simpler, concentrating on the "new" topics, ie C-strings and manual memory management.


So many problems so it's hard to tell you what to fix first, especially since you don't seem to read all the advice given.

But let's start here:
	void Shader::LoadShader(const char* shaderName, const char* path)
	{
		std::ifstream file;
		file.open(path);
		if (file.good() == false)
		{
			Logging::Log("Failed to open shader file : " + std::string(path));
			strcpy(m_shaderSource, "NULL");
			strcpy(m_shaderName, "NULL");
			strcpy(m_shaderPath, "NULL");
			return;
		}



Where have you allocated memory for those pointers (m_shaderSource, etc.)? And why are you using pointers instead of statically allocating the memory for these variables?

Why are you trying to learn two difficult concepts at the same time (manually allocating memory and using C-strings). Since you have such a large program I really suggest you keep things simple, use std::strings instead of the C-strings. If you want to learn C-strings then create a small program to play with C-strings until you think you understand the concept, the same with dynamic memory allocation create a small program to just play with this concept until you think you understand the concept. Trying to learn new concepts inside a "large" program can be very confusing.

		m_shaderSource = (char*)malloc(shaderSourceStr.size());
		m_shaderName = (char*)malloc(strlen(shaderName));
		m_shaderPath = (char*)malloc(strlen(path));
		strcpy(m_shaderSource, shaderSourceStr.c_str());


Why are you using malloc instead of new? And be careful you're not allocating enough memory for these strings.

Jim
Was This Post Helpful? 0
  • +
  • -

#14 The Shroom  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 21
  • Joined: 06-April 16

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 02:19 PM

Is there really a need to post all of the code, including the one unrelated to my problem though?
Here are all the files in my project :

Spoiler


I really thought I understood memory management until I started this topic, seems like I was very wrong.
I have watched several topics and videos about memory management, I think I know how the stack/heap/global/text works.
However none of the videos or topics I have found have explained specifically how char pointers, or the c-related functions like malloc work.

What do you mean by "not reading all advice given"?
I do know that your advising me to use std::string instead of char* and to use new instead of malloc, but I want to learn how to use those. (from now on I will do it in a separate project though, like you said)

I had not allocated memory for those pointers (I think I have allocated it correctly now though), and I used malloc because I wanted to learn how that works as well, is there any downside to using malloc compared to new?
And again, I used pointers since I wanted to learn about char pointers.
Was This Post Helpful? 0
  • +
  • -

#15 jimblumberg  Icon User is offline

  • member icon

Reputation: 5245
  • View blog
  • Posts: 16,297
  • Joined: 25-December 09

Re: Why does my C-String display weird symbols?

Posted 29 June 2017 - 03:46 PM

Quote

However none of the videos or topics I have found have explained specifically how char pointers, or the c-related functions like malloc work.

A pointer is a pointer is a pointer. Your problem with char pointers is not really related to the pointer itself but rather related to C-strings. You're attempting to learn two distinct topics at the same time, C-strings and manual memory management. Before you go the manual memory management route you really need to understand how C-strings and the various C-string functions work. Once you really understand C-strings then using manual memory management for C-strings will be much easier to understand. Also, IMO, part of your problem is that you don't really understand arrays (yet another distinct topic).

Quote

I had not allocated memory for those pointers (I think I have allocated it correctly now though)

How many times is it possible for this function to be called? If it can be called more than once you will have a memory leak.

	void Shader::LoadShader(const char* shaderName, const std::string& path) 
		std::ifstream file(path); // Prefer using the constructor instead of calling open, and note "modern" C++ can use a std::string instead of a C-string.

		if (!file.good())
		{
			Logging::Log("Failed to open shader file : " + path); // Just pass a string for the path variable instead of converting the C-string to a string.
	//		char* txt = "NULL";  // This really should be const qualified, and you would be better off using array notation:
const char txt[] = "NULL"; // This is a compile time constant C-string that is local to this block, no dynamic memory involved.

			//m_shaderSource = new char[strlen(txt) + 1]; // If you use the above syntax you can use:
m_shaderSource = new char[sizeof(txt)];  // Note, no addition is necessary.

			m_shaderName = new char[strlen(txt) + 1];
			m_shaderPath = new char[strlen(txt) + 1];

			strcpy(m_shaderSource, "NULL");
			strcpy(m_shaderName, "NULL");
			strcpy(m_shaderPath, "NULL");
			delete txt;           // This is wrong. You can't delete or free memory you don't new or malloc.
			txt = nullptr;        // Again this is wrong, and it is not needed anyway. This variable goes out of scope at when the function returns.
			return;
		}

		std::string shaderSourceStr{ std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>() };
//		m_shaderSource = new char[strlen(shaderSourceStr.c_str()) + 1]; // No need for strlen() here, a string knows it's size.
m_shaderSource = new char[shaderSourceStr.size() + 1];
		m_shaderName = new char[strlen(shaderName) + 1];
		m_shaderPath = new char[strlen(path) + 1];
		strcpy(m_shaderSource, shaderSourceStr.c_str());
		strcpy(m_shaderName, shaderName);
		strcpy(m_shaderPath, path);

	//	file.close(); // No need to manually close the file, the file will be closed when the stream goes out of scope.
	}



Quote

, and I used malloc because I wanted to learn how that works as well, is there any downside to using malloc compared to new?

Yes there are downsides to using malloc in C++, first malloc() is not considered type safe and you must cast the return value to the proper type. This casting can cause hard to find bugs because compilers usually assume you know what you're doing and don't always issue diagnostics for potential misuse.

Quote

but I want to learn how to use those.

If you really want to learn those C topics maybe you should be learning C instead of C++. Even though C and C++ are related they are quite difference and using a lot of the C standard features are not necessarily good ideas in C++.

Quote

What do you mean by "not reading all advice given"?

I suggested using the proper C++ techniques, ie new and std::string to make getting your "large" program working correctly easier. C++ strings are much less error prone, they always know their size, can be compared using the "normal" comparison operators, and you don't need to worry about that pesky string termination character. Because of all of these reasons (and more) they help to eliminate a very large portion of the buffer overrun errors that plague C programs.

Jim
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2