Manage risk of "Overflowing fixed-length string buffers"

I want to verify that I'm correctly handling risk of "Overflowing fixed-length string buffers." I'm also aware that I'm using C-style strings, which falls short of full C++ code.

Main

```
/*
PURPOSE:
attempt to implement, without using given/known-good code, various concepts
of c++.

one exception:
code in header fileToArray/set_cArrayTemp()
*/

//MOVED TO TOP - ELSE MY HEADERS FAIL
//boilerplate - won't have to use: std::
using namespace std;

//needed to use cout/cin
#include

//for using system calls -
//warning: (www.cplusplus.com/forum/articles/11153/)
#include

//to use strlen
#include

//c++ headers (e.g. iostream) don't use .h extension so i am not either

//provides indentation and v spacing for logging
#include "header/logFormatter"

//reads a file to a char array
#include "header/fileToArray"

//dialog gets filename from user
#include "header/fileDialog"

//this is the (p)array that should hold all the text from the file
char *cArray;

//the name of the file to read
char *fileName;

void set_fileName(){
cout << vspace << col1 << "begin set_fileName()";
char *temp = getFileName();
cout << col2 << "tmp: " << temp;
fileName = new char[strlen(temp)];
strcpy(fileName,temp);
delete[] temp;
cout << col2 << "fileName is set: " << fileName;
cout << col1 << "end set_fileName()";
}

void set_cArray(){
cout << vspace << col1 << "begin set_cArray()";
char *temp = fileToArray(fileName);
if(temp){
cout << col2 << "tmp: " << temp;
/*FROM MAN STRCPY:
"If the destination string of a strcpy() is not large enough,
then anything might happen. Overflowing fixed-length string
buffers is a favorite cracker technique for taking complete
control of the machine."
*/
//so, this guards against overflow, yes?
cArray = new char[strlen(temp)];
strcpy(cArray,temp);
delete[] temp;
cout << col2 << "cArray is set: " << cArray;
cout << col1 << "end set_cArray

It is very apparent that you are new. That is fine; we were all new once. I'll try to adjust my feedback accordingly. Let me know if you are not familiar with some of the terminology, and I'll provide an explanation or definition.

Overflow safeguards

First of all, I'll discuss what seems to be the issue you are most concerned with: buffer overflows. I'd like to state that at this level, you should not really care about that (yet), at least not from a security standpoint. You should focus on learning the language and programming in general first.

Your code is following the general correct idea: Make sure buffers are large enough by dynamically allocating memory, and limiting the size of input strings when you are not. Note that std::string does all of this for you by growing as needed.

In modern C++ code, you would normally avoid allocating buffers the way you do, because memory management quickly becomes hard as a program grows. In C++, the RAII pattern is essential. It boils down to allocating resources in the constructor of an object (a class instance), and freeing them automatically in the destructor when the object goes out of scope. This is what std::string does for you, as well as growing as needed if you add text to the string.

High-level issues

1. I strongly recommend you to reduce the commenting level.

I used to teach programming at the local university, and I saw that over-commenting "technique" a lot. My experience is that it not a good idea. It works as a crutch, allowing you to read your comments rather than your code. However, you already know how to read text; you need to learn how to read code. Stick to regular commenting levels. If you must have notes, keep them in a separate document. You want to make it as inconvenient as possible for you to look at them, forcing you to read the code itself when possible.

1. You are not writing C++.

You are writing C code, with C++ library calls. Write your own String class instead of using raw arrays in the code. Take advantage of all the things C++ has to offer. (This normally includes std::string, but writing your own String class for practice is a nice exercise.)

1. Your headers should not contain function definitions.

In C++, there is something called the one definition rule, which states that any definition should occur at most once in a program. (There are some exceptions to this, but you don't have to think about that yet.) Headers are meant to be included in several files, so they can only have declarations in them. For example:

In fileDialog.hpp:

char *getFileName();

In fileDialog.cpp:

char *getFileName(){
    cout << vspace << col2 << "begin getFileName()";
    char* input = new char[size];
    cout << col2 << "enter filename: ";
    cin.getline(input,size);
    cout << col2 << "end getFileName()";
    return input;
}

The former is a declaration, the latter a definition. While we're on the subject of headers: It's normal for user-defined headers to have a .h, .hpp or .hxx suffix. I personally prefer .hpp to separate them from C headers.

1. Avoid using global variables.

Global variables are bad, because they have a very large scope and can be changed from anywhere, at any time, sometimes without you realizing. Either implement a class design and put the variables into class scope, or pass them around using function arguments. Variables that will never change (often called constants :-) ) can be left in the global scope, but should be declared const.

1. Learn the basics of a debugger.

Basic use of a debugger is very simple, and it allows you to remove a lot of the cout calls that clutter the code. Learn to set breakpoints and step through your code; that's all you need for now. As a beginner, I recommend using a visual debugger and not just raw gdb. (You can use a gdb frontend, though.)

1. Separate output from computations.

Functions that do something should generally not perform IO. One of the key reasons for that is reusability. You want to write code that you can reuse later. While it's not very likely that you will reuse these functions later, you should train as you fight and follow good practice whenever possible. Later users of your functions (i.e. you at a later time) may not want that output, and the way to solve that is to decouple IO from computations.

Lower-level issues

1. It's safe to delete a null-pointer.

Instead of writing

if(cArray){
    delete[] cArray;

Write

delete [] cArray;
cArray = nullptr;

deleteing a null-pointer has zero effect, and is therefore harmless, so there's no point in checking against null. What you should do, however, is to set your pointer to null after deleting it, ensuring that nothing bad will happen if it is deleted again. In C++11, the null pointer is called nullptr. If for some reason you are not using C++11 (as a C++ learner in 2013, you should be), use 0 (or NULL) instead.

1. Consider inverting conditi

StackExchange Code Review Q#28429, answer score: 5