FILE - creating files...

[jvqpDjzh]

/*
Why this program doesn't create the file???
*/

#include <iostream>
#include <cstring>
using namespace std;

int main(int argc, char** argv) {

FILE* file1 = new FILE();

string path = "C:\\Users\\User1\\Desktop\\";
string name1 = "file1";
string extension = ".txt";

const char * address1 = (path + name1 + extension).c_str();

file1 = fopen(address1, "w");

return 0;
}

[AbstractionAnon]

Because it won't compile?

Line 17 makes no sense. You can't combine strings like that and then call c_str().

PLEASE USE CODE TAGS (the <> formatting button) when posting code. It makes it easer to read you post and to respond to it.

[jvqpDjzh]

Ok, i will do it ;)

It compiles, but doesn't create nothing...

So why does c_str() exist? And why can't I combine the strings in this way?

[Catfish666]

1) Please use code tags when posting code. Read this article for more information:
http://www.cplusplus.com/articles/jEywvCM9/

2) That's not you use FILE (you don't new it -- and then also forget to delete it).

Here's an example for using FILE:
http://www.cplusplus.com/reference/cstdio/FILE/

3) Being a C++ programmer you should be using C++ file streams instead of C-style I/O:
http://www.cplusplus.com/doc/tutorial/files/

Finally to answer your question: maybe you do not have rights to create a file in that directory? Does the directory exist? In fact does your program even compile, without having included cstdio?

Line 17 makes no sense. You can't combine strings like that and then call c_str().

Well it does make sense. The result of combining the strings will be a temporary string, which is why it compiles fine (I'm not sure how safe it is though).

[MikeyBoy]

AbstractionAnon wrote:

Because it won't compile?

Why on earth not?

It is perfectly legal to use the + operator to concatenate std::strings together:

http://www.cplusplus.com/reference/string/string/operator+/

Since the result of that concatenation is, itself, a std::string, it's perfectly legal to call the c_str() method on it.

The problem is that, in the OP's code, (path + name1 + extension) is a temporary object. There's no point storing the pointer to the char array like that, because the memory it's pointing to will become invalid when the temporary object is destroyed.

So, it's legal C++, but it will result in a runtime error.

The following would, however, work just fine:

string path = "C:\\Users\\User1\\Desktop\\";
string name1 = "file1";
string extension = ".txt";

char address1[256];  // Assuming the total length of the string is not greater than 255 chars
strcpy(address1, (path + name1 + extension).c_str());

file1 = fopen(address1, "w");

EDIT: Yes, I've ignored any attempt to make that strcpy safe from running over the end of the array. I'm illustrating a point, not writing production code.

[Catfish666]

There's no point storing the pointer to the char array like that, because the memory it's pointing to will become invalid when the temporary object is destroyed.

The question is when exactly will the temporary object be destroyed. If it will die only when the function returns then all should be fine.

[MikeyBoy]

The question is when exactly will the temporary object be destroyed. If it will die only when the function returns then all should be fine.

But then we're in the realm of undefined behaviour. There's no guarantee in the standard that the temporary object will remain in existence for longer than the statement in which it's required.

And you wouldn't recommend a user relies on undefined behaviour, would you? :P

[jvqpDjzh]

char address1[256]; // Assuming the total length of the string is not greater than 255 chars

MikeyBoy (2167)

I was trying to use the pointer to chars because of course I don't know initially the length of the path...

the memory it's pointing to will become invalid when the temporary object is destroyed.

MikeyBoy (2167)

Sorry, what do you really mean with "the temporay object is destroyed"?

[jvqpDjzh]

(you don't new it -- and then also forget to delete it)

Catfish666 (558)

Of course I "new" it.

FILE* file1 = new FILE();

It would work also if i do not delete it...

Being a C++ programmer you should be using C++ file streams instead of C-style I/O:

Catfish666 (558)

I use the C-style because it's the only way i know and it seems to be easy to work with.
Yes, when i will be a good programmer i will know to handle both ways of doing it ;)

Please use code tags when posting code. Read this article for more information:

sorry, but i think that in this case its not so complicated to undestand what i ask...

[MikeyBoy]

I was trying to use the pointer to chars because of course I don't know initially the length of the path...

Well, it should be trivial to add stuff to what I posted to handle that aspect.

Sorry, what do you really mean with "the temporay object is destroyed"?

The object that you are calling c_str() on, is the object that results from evaluating (path + name1 + extension). That object is a temporary object. It is not guaranteed to exist for any longer than the statement in which it is used.

The pointer returned by your c_str() call is, therefore, a pointer into memory that is managed by a temporary object. By the time you use it in your call to fopen(), that object may have been destroyed, so that memory may no longer be valid.

Of course I "new" it.

You do, but you shouldn't. That's not how you use C-style file handling functions. The fopen() function is responsible for allocating the data structure, and you just need to store the pointer that it returns.

But, as Catfish says, you'd be much better off using the C++ style of doing things.

[jvqpDjzh]

This code works! But if I save first the strings objects in 1 string object and then convert it to a C-string, the problem you mentioned disappears?

	FILE* file1;//not newing it
	
	string filePath = path + name + extension;		

	const char * const address1= filePath.c_str();	

	if (file1 != NULL)
		file1 = fopen(address1, "w");
	else
		file1 = fopen(address1, "r");
	
	fclose(file1);

[MikeyBoy]

This code works! But if I save first the strings objects in 1 string object and then convert it to a C-string, the problem you mentioned disappears?

Exactly. You are now creating your own object on the stack, filePath, as a copy of the temporary object. filePath persists for as long as it remains in scope, which means that your address1 pointer is pointing to valid memory for as long as filePath is in scope.

The following, however, doesn't make any sense:

	if (file1 != NULL)
		file1 = fopen(address1, "w");
	else
		file1 = fopen(address1, "r");

At the point where you have that if statement, file1 is uninitialised, which means the value is undefined. It might be NULL, or it might be something else, but that that depends entirely on the C++ compiler you're using.

Its value certainly has nothing to do with whether or not the file exists; how could it, when you haven't yet done anything to associate file1 with any particular file?

Using the uninitialised value of file1 to determine the behaviour of your program makes no sense.

[Smac89]

try to stick with one, either c or c++

#include <stdio.h>

int main(int argc, char** argv) {

    char path[] = 
    {
        "C:\\Users\\User1\\Desktop\\"
        "file1"
        ".txt"
    };

    FILE *file = fopen(path, "w");
    if (NULL == file) perror("Operation failed");
    else puts("Success!");

    fclose(file);
 
    return 0;
}

#include <fstream>
#include <iostream>
#include <string>

int main(int argc, char** argv) {

    std::string path = std::string("C:\\Users\\User1\\Desktop\\") + "file1" + ".txt";

    std::ofstream file(path.c_str());
    if (!file.good()) std::cerr << "Operation failed";
    else std::cout << "Success!";

    file.close();
 
    return 0;
}

[jvqpDjzh]

//I know this makes any sense, i am fucking stupid! Thank you for made me noticed it again!

	if (file1 != NULL)
		        file1 = fopen(address1, "w");
	        else
		        file1 = fopen(address1, "r");

I would like to say: if it doesn't exist create it! (What would be your simple solution?)

Omg, I am fucking stupid: if (file1 != NULL) ? NO! It has really no sense, I don't know why I continue to try to be a programmer...

At most, if (file1==NULL) assuming I initialize: FILE* file1 = NULL;

[Smac89]

Actually in most code you will come across, the way you have written your if statement is the way those programs are structured:

int main(int argc, char **argv) {

    FILE *file = fopen( /* somefile here */);

    if (file1 != NULL) {
        // long piece of code here



    }
    // somewhere down here
    else {
        // some clean up code here...



        perror ("Operation failed, exiting...");
        return 1;
    }

    return 0;
}

This way the person reading the code can dive right in and see what the program is doing rather than first seeing the cleanup

[jvqpDjzh]

This is my code and works...Suggestions?

/*
main.cpp
*/

#include "PhoneBook.h"
#include <iostream>
#include <cstring>
using namespace std;


int main(int argc, char** argv) {

	PhoneBook* pb = new PhoneBook();
	
	const char * const filePath = pb->ToPointer("C:\\Users\\User1\\Desktop\\", "file1", ".txt");
	pb->createFile(filePath);

	delete pb;
	return 0;
}

/*
PhoneBook.h
*/
#include <cstring>
#include <iostream>
#include <cstdlib>
using namespace std;

class PhoneBook
{
	private:
		string path;
		string name;
		string extension;
	
	public:
		bool createFile(const char * const filePath);
		bool deleteFile(const char * const filePath);
		const char * const ToPointer(string path, string name, string extension);
		
		PhoneBook();	
		~PhoneBook();
};

/*
PhoneBook.cpp
*/

#include "PhoneBook.h"

PhoneBook::PhoneBook()
{
	path = "C:\\Users\\User1\\Desktop\\";
	name = "file1";
	extension = ".txt";	
}

PhoneBook::~PhoneBook()
{
}

const char * const PhoneBook::ToPointer(string path, string name = "cfile", string extension = ".txt")
{
	string filePath = path + name + extension;		
	const char * const address1= filePath.c_str();
	
	return address1;		
}

bool PhoneBook::createFile(const char * const filePath)
{
	
	FILE* file1 = fopen(filePath, "w");
	
	if (file1 != NULL) 
		cout << "File created successfully!\n";		
	else 
		cerr << "ERROR: impossible to create the file.\n";		
	
	fclose(file1);		

}

bool PhoneBook::deleteFile(const char * const filePath)
{
	
	if (remove(filePath) != 0)
		cerr << "ERROR: impossible to remove the file. Retry." << endl;
	else
		cout << "File removed successfully!" << endl;
}

[Smac89]

class PhoneBook
{
	private:
		FILE *file;
	
	public:		
		PhoneBook(); // creates default file
                explicit PhoneBook(const char *); // uses the string to create a new file
		~PhoneBook();
};

[jvqpDjzh]

Thanks!

[AbstractionAnon]

You still have the problem of returning an invalid pointer.

const char * const PhoneBook::ToPointer(string path, string name = "cfile", string extension = ".txt")
{
	string filePath = path + name + extension;		
	const char * const address1= filePath.c_str();
	
	return address1;		
}

When you exit the function ToPointer(), filePath goes out of scope and it's contents are no longer valid. Therefore, address1 points to garbage because the contents of filePath are no longer on the stack.

Try this instead:

string PhoneBook::ToFilename(string path, string name = "cfile", string extension = ".txt")
{   return path + name + extension;		
}

[jvqpDjzh]

Thanks.