deallocating char** (double pointer) properly.

[Xegros]

hi there, i'm trying to get this get to work but after some clicks (the procedure activates after a click) some text gets messed up, it is some memory mess i guess. anyway take a look, tell me if there's anything wrong (i'm using C++Builder).
void __fastcall TForm1::btnTraceClick(TObject *Sender)
{
String options;
//options = Application->ExeName;
options = "KanedaX";
int MaxWidth=0,i,argc=1;
char **argv=NULL;
char *options_c,*str;

lstboxFiles->Clear();
//OPTIONS
if (comboFormat->Items->Strings[comboFormat->ItemIndex] != "SVG") {
options += "|-b|" + comboFormat->Items->Strings[comboFormat->ItemIndex];
argc += 2;
}
if (!editOutput->Text.IsEmpty()) {
options += "|-o|" + editOutput->Text;
argc += 2;
}
//DONE with OPTIONS
//copy String options -> char *options_c
options_c = (char *)malloc(options.Length()+1);
strcpy(options_c,options.c_str());
//start tokenizing
str = strtok(options_c,"|");
//memory for array of char *
argv = (char**)malloc(argc);
for (i=0; i<argc; ++i) {
//copy each str to argv[i]
argv[i] = (char *)malloc(strlen(str)+1);
strcpy(argv[i],str);
//free mem for both str & options_c (first portion)
// free(str);
//continue tokenizing
str = strtok(NULL,"|");
}
free(options_c);
//show argv on ListBox
for (i=0; i<argc; ++i) {
if (MaxWidth < Canvas->TextWidth(argv[i]))
MaxWidth = Canvas->TextWidth(argv[i]);
lstboxFiles->Items->Add(argv[i]);
}
//Free mem. for argv
for (i=0; i<argc; ++i) {
free(argv[i]);
argv[i] = NULL;
}

//fix it: memory doesn't get all freed.
// can't figure out why.

//Adjust Width
SendMessage(lstboxFiles->Handle,LB_SETHORIZONTALEXTENT,
MaxWidth + 5, 0);
}

[gpraghuram]

hi there, i'm trying to get this get to work but after some clicks (the procedure activates after a click) some text gets messed up, it is some memory mess i guess. anyway take a look, tell me if there's anything wrong (i'm using C++Builder).
void __fastcall TForm1::btnTraceClick(TObject *Sender)
{
String options;
//options = Application->ExeName;
options = "KanedaX";
int MaxWidth=0,i,argc=1;
char **argv=NULL;
char *options_c,*str;

lstboxFiles->Clear();
//OPTIONS
if (comboFormat->Items->Strings[comboFormat->ItemIndex] != "SVG") {
options += "|-b|" + comboFormat->Items->Strings[comboFormat->ItemIndex];
argc += 2;
}
if (!editOutput->Text.IsEmpty()) {
options += "|-o|" + editOutput->Text;
argc += 2;
}
//DONE with OPTIONS
//copy String options -> char *options_c
options_c = (char *)malloc(options.Length()+1);
strcpy(options_c,options.c_str());
//start tokenizing
str = strtok(options_c,"|");
//memory for array of char *
argv = (char**)malloc(argc);
for (i=0; i<argc; ++i) {
//copy each str to argv[i]
argv[i] = (char *)malloc(strlen(str)+1);
strcpy(argv[i],str);
//free mem for both str & options_c (first portion)
// free(str);
//continue tokenizing
str = strtok(NULL,"|");
}
free(options_c);
//show argv on ListBox
for (i=0; i<argc; ++i) {
if (MaxWidth < Canvas->TextWidth(argv[i]))
MaxWidth = Canvas->TextWidth(argv[i]);
lstboxFiles->Items->Add(argv[i]);
}
//Free mem. for argv
for (i=0; i<argc; ++i) {
free(argv[i]);
argv[i] = NULL;
}

//fix it: memory doesn't get all freed.
// can't figure out why.

//Adjust Width
SendMessage(lstboxFiles->Handle,LB_SETHORIZONTALEXTENT,
MaxWidth + 5, 0);
}

Hi,
You havent freed the following memory

argv = (char**)malloc(argc); // This is not freed
for (i=0; i<argc; ++i) {
   free(argv[i]);
   argv[i] = NULL;
 }
free argv; //add this line to free the argv memory
 

Raghuram

[dumparun]

Hi,
You havent freed the following memory

argv = (char**)malloc(argc); // This is not freed
for (i=0; i<argc; ++i) {
   free(argv[i]);
   argv[i] = NULL;
 }
free argv; //add this line to free the argv memory
 

Raghuram

this is how you allocate memory

         variable = new char*[X];
 
     for( i = 0; i <  X + 1; ++i)
        variable[i] = new char[Y];

[Xegros]

Thanks to everybody for the help. But you know what, while i was sleeping (i've just had a 2 hour nap) i had an illumination. I realized that that i had to do this :

argv = (char**)malloc(argc*4);

instead of just this :

argv = (char**)malloc(argc);

Man i can't believe it was basic stuff. One of you said i forgot to free the memory for "argv", well i actually didn't forget, desperate as i was i started testing with everything i could.

Anyway thanks to everybody for the quick reply!

[weaknessforcats]

Everyone is making this too hard. There are no multi-dimensional arrays in C or C++. All there are are one dimensional arrays. The first index specifies the number of elements:

Code: ( c )
int arr[3]; //has 3 elements. Each element an int
int arr[3][7]; //has 3 elements. Each element an array of 7 int
int arr[3][4][5]; //has 3 elements. Each element an array 4 arrays of 5 ints.


Further, arrays must be contiguous in memory. Element 0 then element 1, etc.

So you allocate int arr[3][4[5] as:


Code: ( c )
int* arr = malloc(3 * 7* sizeof(int));


You now have 21 ints. If you want to use this as a 2D array then tou need to cast the return from malloc to the correct pointer. In this case, a pointer to a an array of 7 int:


Code: ( c )
int (* arr)[7] =(int (*)[7] )malloc(3 * 7 * sizeof(int));


Remember, the name of an array is always the address of element 0. In this case the elements are arrays of 7 ints. An int** is a pointer to a pointer to a single int. Trying to use this as a 2D array will produce the wrong address calculations.