Read/Write file C&Cpp -XOR Encryption -compile/logic err

[MasterGod]

Phew, I got it all in the subject (I hope..)

I'm trying to make 2 functions, one in C and one in C++ that reads a text file and doing XOR Encryption on it.
1. I get compilation errors with the C++ code.
2. The C code gets into a infinite loop and I know why but I can't fix it.

Main:

Code: Select all

#include <stdio.h>
#include <conio.h>
#include <stdlib.h>
#include <string.h>
#include <fstream>

using namespace std;

int XOR_Encryption_C(char *, char []);
bool XOR_Encryption_CPP(char *, char []);

void main()
{
	// clrscr();
	char *file_Path = (char *)calloc(sizeof(char), 255);

	printf("Enter file's path (C):\n");
	scanf("%s", file_Path);
	if (!XOR_Encryption_C(file_Path, "a"))
	{
		printf("Error! (C)");
		getch();
	}
	printf("Enter file's path (C++):\n");
	scanf("%s", file_Path);
	if (!XOR_Encryption_CPP(file_Path, "a"))
	{
		printf("Error! (C++)");
		getch();
	}
}

C Version:

Code: Select all

int XOR_Encryption_C(char *FilePath, char key[])
{
	FILE *fp = fopen(FilePath, "r+"); // The file must exist
	if (fp)
	{
		char ch;
		int i = 0;
		while((ch = fgetc(fp) != EOF))
		{
			if (i < strlen(key))
				ch ^= key[i++];
			else
			{
				i = 0;
				ch ^= key[i++];
			}
			fseek(fp, -1L, SEEK_CUR); // DOESN'T WORK!!!
			fputc(ch, fp);
		}
		fclose(fp);
	}
	else
		return 0;

	return 1;
}

fseek(fp, -1L, SEEK_CUR); suppose to go one step back right? well, it doesn't, it gets to the beginning of the file every time.
If this is fixed the whole C version should be working just fine.

C++ Version:

Code: Select all

bool XOR_Encryption_CPP(char *FilePath, char key[])
{
	char ch;
	int i = 0;
	fstream fp(FilePath, ios::in | ios::out | ios::nocreate);
	while(!fp.eof())
	{
		fp.get(ch);
		if (i < strlen(key))
			ch ^= key[i++];
		else
		{
			i = 0;
			ch ^= key[i++];
		}
		fp.seekp(fp.tellg()-1);
		fp << ch;
	}
	fp.close();

	return true;
}

This should do the same thing but I can't debug it to see if it does cause it doesn't compile. This is what I get:

Code: Select all

------ Build started: Project: test, Configuration: Debug Win32 ------
Compiling...
XorEncryption.cpp
.\XorEncryption.cpp(64) : error C2039: 'nocreate' : is not a member of 'std::basic_ios<_Elem,_Traits>'
        with
        [
            _Elem=char,
            _Traits=std::char_traits<char>
        ]
.\XorEncryption.cpp(64) : error C2065: 'nocreate' : undeclared identifier
.\XorEncryption.cpp(75) : error C2666: 'std::fpos<_Statetype>::operator -' : 3 overloads have similar conversions
        with
        [
            _Statetype=_Mbstatet
        ]
        D:\Program Files\Microsoft Visual Studio 8\VC\include\iosfwd(98): could be 'std::fpos<_Statetype> std::fpos<_Statetype>::operator -(std::streamoff) const'
        with
        [
            _Statetype=_Mbstatet
        ]
        D:\Program Files\Microsoft Visual Studio 8\VC\include\iosfwd(75): or 'std::streamoff std::fpos<_Statetype>::operator -(const std::fpos<_Statetype> &) const'
        with
        [
            _Statetype=_Mbstatet
        ]
        or 'built-in C++ operator-(std::streamoff, int)'
        while trying to match the argument list '(std::fpos<_Statetype>, int)'
        with
        [
            _Statetype=_Mbstatet
        ]
Build log was saved at "file://d:\Programming\Projects\Active Projects\Visual Studio 2005\Projects\test\test\Debug\BuildLog.htm"
test - 3 error(s), 0 warning(s)
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Using Microsoft Visual C++ 2005 Express Edition.
Any criticism on the code itself, suggestions to make it better are welcome!

Thanks to everyone who will help me with this problem, I've been trying to fix it myself for few hours now, I hope I'll finally get my answers.. *relief*

[shogun]

fseek(fp, -1L, SEEK_CUR); suppose to go one step back right? well, it doesn't, it gets to the beginning of the file every time.

Which position it it at the beginning of your function?

Code: Select all

------ Build started: Project: test, Configuration: Debug Win32 ------
Compiling...
XorEncryption.cpp
.\XorEncryption.cpp(64) : error C2039: 'nocreate' : is not a member of 'std::basic_ios<_Elem,_Traits>'

I guess 'nocreate' is not a member of ios.

[MasterGod]

fseek(fp, -1L, SEEK_CUR); suppose to go one step back right? well, it doesn't, it gets to the beginning of the file every time.

Which position it it at the beginning of your function?

?? The beginning of the file, some memory address? or maybe I didn't understand what you're asking?..

[shogun]

I mean, you're trying to go backwards through your file. Unless I'm not mistaken (I don't write C anymore), you probably have to call

Code: Select all

fseek(fp, 0, SEEK_END);

somewhere at the beginning of the function.

[vitek]

No, that is not the problem. fgetc() moves the file position indicator forward by one unsigned character. He then seeks back one character and overwrites it with an fputc() call, which again increments the file position indicator.

The problem is that he didn't read the documentation for the functions he is using...

When the "r+", "w+", or "a+" access type is specified, both reading and writing are allowed (the file is said to be open for "update"). However, when you switch between reading and writing, there must be an intervening fflush, fsetpos, fseek, or rewind operation. The current position can be specified for the fsetpos or fseek operation, if desired.

So, between the call to fputc() and fgetc(), you need a call to fflush(). It might also be a good idea to do some error checking.

Also, if runtime performance becomes an issue, it might be a good idea encode a block of characters [or the entire file] instead of doing one byte at a time. It might also be a good idea to avoid encoding the file in place in case some error does come up. If you leave the file partially encoded, it may be difficult to recover the data.

Travis

[JP]

Yes, it's definetly best to read the whole file into a buffer first and then iterate through the buffer instead of reading each byte from the file as it's very slow to do that.

I had some model loading code from someone and they'd read each piece of model information from the file one at a time so each vertext coordinate, texture coordinate, index etc. As you can imagine this is a HUGE number of file reads if you have anything with more than a few polys in it. It used to take a fair few seconds to load a model until i changed it to read into a buffer which cut it to just a few milliseconds!

It does make things a bit more complicated but it's much quicker!

Possibly if you're just reading a small text file though it won't be so necessary as there may be relatively few file reads.

[MasterGod]

Ok thanks, now about the C++ version, what's wrong with that?

[vitek]

Well, technically you should only need one implementation, but I'm assuming you're doing this to learn how to use C++ streams.

As you should have learned by now, compile errors are usually easy to resolve if you look at the appropriate headers or documentation. If you look at the declaration of std::ios, you probably won't find any of the openmode flags. If you go from there to its base class std::ios_base, you'll find that there is no std::ios_base::nocreate. So either the documentation that you are using is out of date or for a version of the C++ Standard library that supports this flag as an extension. Recent versions of the Dinkum C++ Standard Library that is distributed with the Microsoft compilers supports this option, but under a different name. You have to use std::ios_base::_Nocreate instead. The weird name is a sign that the code you are writing is not portable between C++ Standard Library implementations.

The second problem is that tellg() returns a std::fstream::pos_type which is the same as std::fpos<...>. The std::fpos<> type is not required to interact well with arbitrary numeric types. It only works with the type std::streamoff. So you need to cast.

Also, it doesn't really help things that you are calling strlen() inside your encoding loop. Might be a good idea to lift that out of there. It will help in debug builds. The compiler may do it for you in release builds if it can prove that the string isn't modified, but why hope that it does when you can avoid the issue entirely.

Travis

[MasterGod]

About working with a block, I'll fix that later but it's a good idea indeed.
About not using the strlen(), I'll fix that later on today cause I must go now.

Well, technically you should only need one implementation, but I'm assuming you're doing this to learn how to use C++ streams.

Yes.

The weird name is a sign that the code you are writing is not portable between C++ Standard Library implementations.

Then how can I make it portable? (if it's even possible..)

Now, I tried what you said and the C version is a infinite loop - don't know why.
The C++ version is also a infinite loop but it writes to the file every time, about 200k per sec.

C

Code: Select all

int XOR_Encryption_C(char *FilePath, char key[])
{
	FILE *fp = fopen(FilePath, "r+"); // The file must exist
	if (fp)
	{
		char ch;
		int i = 0;
		while(!feof(fp))
		{
			ch = fgetc(fp);
			if (i < strlen(key))
				ch ^= key[i++];
			else
			{
				i = 0;
				ch ^= key[i++];
			}
			fseek(fp, -1L, SEEK_CUR);
			fputc(ch, fp);
			fflush(fp);
		}
		fclose(fp);
	}
	else
		return 0;

	return 1;
}

C++

Code: Select all

bool XOR_Encryption_CPP(char *FilePath, char key[])
{
	char ch;
	int i = 0;
	fstream fp(FilePath, ios_base::in | ios_base::out | ios_base::_Nocreate);
	while(!fp.eof())
	{
		fp.get(ch);
		if (i < strlen(key))
			ch ^= key[i++];
		else
		{
			i = 0;
			ch ^= key[i++];
		}
		fp.seekp(-1, ios_base::cur);
		fp << ch;
	}
	fp.close();

	return true;
}

[vitek]

You keep breaking your code. If you add some error checking, you'll see that fgetc() is returning EOF. If you break out the debugger, you'll see that feof() doesn't detect eof when the last operation was a write. That is because, at least on some platforms, the end-of-file bit is not set until the last character is read. If you do a write, the eof flag is reset.

Code: Select all

int XOR_Encryption(const char *file, char* key, int len) 
{ 
  if (len < 1)
    len = strlen(key);

  FILE *fp = fopen(file, "r+b");
  if (fp) 
  { 
    int i = 0; 

    do
    { 
      int chi = fgetc(fp);
      if (chi == EOF)
        break; // reached end of file

      if (! (i < len))
        i = 0; 

      unsigned char chu = unsigned char(chi);
      chu ^= key[i++]; 

      // need to check success of seek
      fseek(fp, -1L, SEEK_CUR); 

      // need to check success of put
      fputc(chu, fp); 

      // need to check success of flush
      fflush(fp); 
    } while (1);

    fclose(fp); 
  } 
  else 
    return 0; 

  return 1; 
}

Travis