Development Kit Class overwriting memory

[PenguinJunk]

I am using the development kit (set of C++ libraries) that comes with some major software that my employer uses. I am using MS Visual Studio 2008 as recommended by the software vendor.

My code includes my own class, ArgFile:

class ArgFile {
    public:
...
	char	*AFileName;
...
	ArgFile();
	~ArgFile();
};

The member, AFileName, can be assigned a value in the function, runBATCHPROC :

char * runBATCHPROC(ArgFile *thisArgFile, ... )
{
...
	char	AFileName[MAXPATHLEN];
	char	thisName[MAXPATHLEN];
...
	sprintf(thisName,"<format>",<values>);
...
	strcat(strcpy(AFileName,thisName),"_end");
	thisArgFile->AFileName=AFileName;
...
}

Currently, the length of the string assigned to thisArgFile.AFileName is less than MAXPATHLEN (1024). When I run another function, processData, parts of the memory allocated to AFileName are overwritten when I declare (but don't assign values to) one particular class from the development kit:

char * processData(ArgFile *thisArgFile, ... )
{
...
    ftr_point_type        pointFtr;
...
}

I have confirmed that this is happening at this stage using the memory window but, because I can't interrogate the classes in the libraries any further, I don't know what part of the pointFtr object is messing up the memory.

Have I done something very silly? Given that I have no ability to change the ftr_point_type class (or any of the devkit classes), can I protect the memory used by other variables?

I have tried to understand what is happening through other forum questions and information pages, but my understanding of memory is too basic to work out how to use the answers given. I would appreciate any hints on how I can investigate the cause of this problem!

Thank you in advance.

[kbw]

public:
    char* AFileName;

??? You could start by implementing data encapsulation properly and make it private.

You're asking for trouble with:

	char	AFileName[MAXPATHLEN];
	char	thisName[MAXPATHLEN];
...
	sprintf(thisName,"<format>",<values>);
...
	strcat(strcpy(AFileName,thisName),"_end");

It should be:

	char	AFileName[MAXPATHLEN];
	char	thisName[MAXPATHLEN];
...
	snprintf(thisName, sizeof(thisName), "<format>", <values>);
...
        strncpy(AFileName, thisName, sizeof(AFileName));
	strncat(AFileName, "_end", sizeof(AFileName));

[PenguinJunk]

Thank you, that's a really useful hint and I will change my code. I can see that snprintf, strncpy, strncat won't prevent the problem I have. I also don't think making AFileName a private member will, will it? I understand that making it private will only stop me overwriting it by stupid calls like ArgFile->AFileName="UtterRot"; but it won't stop the problem I have. I haven't attempted this yet because it will require a lot of changes: there are a number of other cases where AFileName is assigned values and so I'd need to create new variables and of course the get.. set.. functions. I'd love to have perfect code but have to prioritise and my problem with the declared devkit class overwriting parts of AFileName takes precedence at the moment.

One thing that I've never understood is how the location is memory for a variable/member is determined. Before I had to add this additional few lines to set the value of ArgFile.AFileName, its value was set in various places with no problem whatsoever (that I noticed!). However, in this case, it is always written to the same address in memory which seems to coincide to the addresses used by the newly declared ftr_point_class object. Why is this? Why do they both want the same bit of memory? Is there anyway of protecting the memory used by my own classes/variables from being used by classes and variables I cannot control?

[PenguinJunk]

To elaborate a little. I think it may appear that I could solve the problem using

strncpy(thisArgFile->AFileName, AFileName, ... );

I tried this. I need to redefine thisArgFile.AFileName as a char[]. This prevents the memory overwrite. Unfortunately, the main way that thisArgFile.AFileName obtains its values is through one of the library's classes which will not work with char[] (in any way that I know of) and so the result of this solution is a problem earlier in the code.

[Moooce]

Without knowing all your code it's tricky to tell but I could be this. If you're assigning the address of AFileName to thisArgFile->AFileName won't this leave a pointer dangling when AFileName goes out of scope at the end of runBATCHPROC?
The next reserved variable will probably end up overwriting some of that memory, and thisArgFile->AFileName will still be pointing to it.

Can you make

char	AFileName[MAXPATHLEN];
char	thisName[MAXPATHLEN];

global variables or use thisArgFile->AFileName=new char[] to allocate some space to copy AFileName into, remembering to delete[] when finished?

either way kbw's comment below is very valid..

[kbw]

You haven't really posted enough to comment on. But it's impossible to maintain sloppy code.

If you make your data public and use unbounded memory operations you've created an unreliable monster. You can't just fix the odd manifestation of an error when the error lies in the structure.

[PenguinJunk]

Thank you thank you thank you. You are of course all correct. Sorry about the delay, I'm fitting this around childcare and perhaps this explains my stupidity. I have started rewriting and all is starting to work just splendidly.

Regarding kbw: "But it's impossible to maintain sloppy code." I do so agree but was only supposed to be roughing out a prototype. However, I now suspect it'll be the final product so I better get it right.

Thank you all again!

[kbw]

I didn't mean to be harsh. C++ has some very simple and basic rules that are easy to follow. But if you break them you're in for a world of pain.