|  | Deleting Pointer Problem (I think) |  | |
| | | Guest |  |
| Posted: Wed Sep 03, 2008 8:58 pm Post subject: Deleting Pointer Problem (I think) |  |
| |  | |
The code compiles but I think there is an issue in either the way I copy the pointer or the way I am deleting it. My main objective is just to take the values in the memory space of argv and copy it to a variable for later reference incased in an object class.
[main.cpp] #include <iostream> #include "switches.h"
using std::cout; using std::cin; using std::endl;
int main (int argc, char * argv[]) { switches Switches(argc, argv);
Switches.showSwitches();
//for(int i = 0; i < argc; i++) // cout << "argv[" << i << "] = " << argv[i] << endl;
return 0; } [/main.cpp]
[object class] #ifndef SWITCHES #define SWITCHES
class switches { public: switches(int new_numArgs, char* new_Args[]); ~switches();
void showSwitches(); private: int numArgs; char * Args[]; };
#endif
#include <iostream> #include "switches.h"
using std::cout; using std::endl;
switches::switches(int new_numArgs, char * new_Args[]) { numArgs = new_numArgs; *Args = *new_Args; }
switches::~switches() { delete Args; }
void switches::showSwitches() { for(int i = 0; i < numArgs; i++) cout << "argv[" << i << "] = " << Args[i] << endl; } [/object class]
This is my first post here so I don't know how these groups work yet, but any help would be appreciated. I know this is some silly simple problem, just trying to re-teach myself some C++ essentials.
Thanks in advance.
-- [ See LINK for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |
| |
| | | Ulrich Eckhardt |  |
| Posted: Thu Sep 04, 2008 1:58 am Post subject: Re: Deleting Pointer Problem (I think) |  |
| |  | |
thothramerkaba@gmail.com wrote:
| Quote: | The code compiles but I think there is an issue in either the way I copy the pointer or the way I am deleting it. My main objective is just to take the values in the memory space of argv and copy it to a variable for later reference incased in an object class.
|
One thing here, which might be a bit complicated to grasp completely: pointers can easily be copied, but what do they represent? The point is that the pointer itself only allows you to access what it points to. Other facets of this, in particular ownership and the way to release ownership correctly, are not encoded into the pointer type itself. That means that if you have a pointer, it doesn't necessarily mean you own what it points to. Further, there is no way to ask a pointer if it points to an object received from malloc(), new or maybe one of automatic (local) or static (global) duration, so you can't tell if you need free(), delete or nothing at all to release it.
So much for the theory, let's look at the code...
| Quote: | int main (int argc, char * argv[])
|
One thing here: this is completely the same as
int main( int argc, char** argv)
I personally find this version clearer, because here you see that argv is just a pointer to a pointer to a char, and it doesn't somehow implicate that it points to an array.
[put ctor and dtor inline]
| Quote: | class switches { public: [ note: arguments passed are argc/argv from main() ] switches(int new_numArgs, char* new_Args[]) { numArgs = new_numArgs; *Args = *new_Args; } ~switches(); { delete Args; }
void showSwitches(); private: int numArgs; char * Args[]; };
|
Big problem here: the memory passed to main() by the implementation is only for use inside the program, but you don't own it. Therefore, it is also not up to you to release that memory, so the 'delete' in the destructor is simply wrong.
Uli
-- [ See LINK for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |
| |
| | | Chris Uzdavinis |  |
| Posted: Thu Sep 04, 2008 1:58 am Post subject: Re: Deleting Pointer Problem (I think) |  |
| |  | |
| Quote: | int main (int argc, char * argv[]) { switches Switches(argc, argv);
|
Ok so far.
| Quote: | #ifndef SWITCHES #define SWITCHES
|
Unrelated, but you might consider making this guard a little more unique to avoid accidental name collision. You could spend hours debugging this if someone happened to also use SWITCHES as an identifier in their code after including your header. Perhaps name it SWITCHES_HPP_ or something. similar.
| Quote: | class switches { public: switches(int new_numArgs, char* new_Args[]); ~switches();
void showSwitches(); private: int numArgs; char * Args[];
}; #endif
#include <iostream #include "switches.h"
using std::cout; using std::endl;
switches::switches(int new_numArgs, char * new_Args[]) { numArgs = new_numArgs; *Args = *new_Args; }
|
This is simply faulty code. Args does not point to anything, and you therefore cannot dereference it or assign to the address that the uninitialized pointer happens to hold.
You either need to store the pointer that was passed in (no dereferencing), or make a copy of the elements into your own std::vector, for example.
| Quote: | switches::~switches() { delete Args; }
|
This delete is a disaster for a few reasons. For one, you didn't allocate the memory and have no business deleting it. Second, the memory is an array and even if you did allocate the array, you're using the wrong delete operator on it. Arrays should be deleted with delete[], or else you have undefined behavior.
If you simply store the pointer that was provided, you should have an empty destructor. If you make a copy of the elements into a local vector, then again you won't need to do anything in your vector because it will cleanup itself.
After seeing this class, I'm still not sure what it is trying to contribute that you don't already have from argv and argc directly.
-- Chris
[ See LINK for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |
| |
| | | Guest |  |
| Posted: Thu Sep 04, 2008 4:25 am Post subject: Re: Deleting Pointer Problem (I think) |  |
| |  | |
it seems to me that what you're trying to solve is the generic 'copying argv' problem. you can use free libraries to do that for example boost program options or work directly with boost shared_array.
you could also use std::vector and std::string together and do something like:
//--------------- #include <vector> #include <string>
class switches { public: switches(int new_numArgs, char* new_Args[]) : numArgs( new_numArgs ) , Args( new_Args, new_Args+new_numArgs ) { } ~switches() { /*nothing to destruct*/ } void showSwitches(); private: int numArgs; std::vector< std::string > Args; }; //---------------
oberve that you don't need to allocate or dealocate anything this way.
if you wanna stick to new/delete and potential resource leaks you would have to allocate space for the member array like in:
//---ctor--- switches( int new_numArgs, char* new_Args[ ] ) : numArgs( new_numArgs ) { Args = new char * [ new_numArgs ]; for ( int i = 0; i < new_numArgs; i++ ) { int len = strlen( new_Args[ i ] ); Args[ i ] = new char[ len + 1 ]; strncpy( Args[ i ], new_Args[ i ], len + 1 ); } } //------
and release them in destructor like: //---dtor--- ~switches() { for ( int i = 0; i < numArgs; i++ ) { delete Args[ i ]; } delete[ ] Args; } //------
you may also want to declare Args using double indirection
char ** Args;
so you wouldn't need to deal with a conversion to array type.
cheers, gil
-- [ See LINK for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |
| |
|
|