asked on

Trouble freeing memory in array of array of structures in C/Apache Module

Hi

I having trouble freeing memory allocated by the following code.

char *sql; // sql string
	int row, col, rows, cols; // row/col interators and total counters
	char *column_name, *column_data;
	oFields **table; // result table
	oMetadata *metadata = (oMetadata *)apr_pcalloc(r->pool, sizeof (*metadata)); // sql call metadata	

        // load sql string
	sql = "select TOP 3 pid, pkey, pupc, pname, pancestor, ptype, ploc from table";
	// execute an odbtp sql call
	table = oExecute(r, sql, metadata, cfg);
	ap_set_content_type(r, "text/html");
	ap_rprintf(r, "1:%s<br>2:%d<br>3:%d<br>", metadata->message, metadata->rowcount, metadata->colcount);
	// check if there was an error
	if (!metadata->isError && metadata->rowcount && metadata->colcount)
	{
		// set rows and columns
		rows = metadata->rowcount;
		cols = metadata->colcount;
		// loop over rows and columns
		for (row = 0; row < rows; row++)
			for (col = 0; col < cols; col++)
			{
				// pull data into local variables
				column_name = apr_psprintf(r->pool,"%s",table[row][col].name);
				column_data = apr_psprintf(r->pool,"%s",table[row][col].data);
				ap_rprintf(r, "%s:%s", column_name, column_data);
			}
	}
	// free memory
	//for (row = 0; row < rows; row++)
		//free(table[row]); // free columns
	// free rows
	//free(table);
	// free metadata
	free(metadata);

Open in new window

If I run as is then this works perfectly. If I uncomment the section starting with free memory, the function no longer works and crashes.

What should I change to properly free the memory? I don't know if it helps but my environment is Apache 2.2 with this code running in a module. I read somewhere that Apache cleans up its own memory, but I am not sure.

Thanks

static oFields **oExecute (request_rec * r, const char* sql, oMetadata *metadata, infsec_config *cfg)
{
	// variables
	odbHANDLE hCon, hQry;
	oFields **table;
	char *connection = apr_pstrdup(r->pool, ""); // connection string
	char *errors = apr_pstrdup(r->pool, ""); // any errors that occur go here
	size_t	col = 0, row = 0, rows = 0, cols = 0;	// row/col count and counter
	int		colType, colLen = 0, isError = 0;
	char	*colName = apr_pstrdup(r->pool, "");
	char	*colData = apr_pstrdup(r->pool, "");
	// create a connection
	connection = apr_psprintf(r->pool, "DRIVER={SQL Server};SERVER=%s;UID=%s;PWD=%s;DATABASE=%s", cfg->infsec_odbtp_serv, cfg->infsec_odbtp_user, cfg->infsec_odbtp_pass, cfg->infsec_odbtp_db);
	// sanity checks
	if (!strlen(sql)){errors = apr_pstrdup(r->pool, "SQL String must not be zero length!"); goto complete; } // sql string must be valid
	// initalize odbtp network library
	if ( !odbWinsockStartup() ){ errors = apr_pstrdup(r->pool, "Could not initiate network transfer."); goto complete; }
	// check that we can connect
	if( !(hCon = odbAllocate(NULL) ) ) { errors = apr_pstrdup(r->pool, "Could not allocate connection"); goto complete; }
    if( !odbLogin(hCon, cfg->infsec_odbtp_link, 2799, ODB_LOGIN_NORMAL, connection )) { errors = apr_pstrdup(r->pool, "Could not login"); goto complete; }
    if( !(hQry = odbAllocate( hCon ) ) ) { errors = apr_pstrdup(r->pool, "Could not allocate query"); goto complete; }
	if( !odbExecute( hQry, sql ) ){  errors = apr_psprintf(r->pool, "Could not execute query: %s", odbGetErrorText( hQry )); goto complete; }
	// check for data --> no data does not imply an error
	if ( odbNoData( hQry ) )
	{ 
		// copy error buffers
		errors = apr_psprintf(r->pool, "%s", odbGetErrorText( hQry ));
		// check if there were any errors
		if (strlen(errors) && errors != "(null)")
			isError = 1;
		// exit
		goto complete; 
	}
	// retrieve column count
	cols = odbGetTotalCols( hQry ); // column count
	// read from stream
	if (cols)
	{
		// intialize the table
		table = (oFields **)0;
		// loop over rows to get data
		while( odbFetchRow( hQry ) && !odbNoData( hQry ) ) 
		{
			// initialize the table for a row
			table = realloc(table,(rows+1) * sizeof (oFields *));
			if (table == NULL){ errors = apr_pstrdup(r->pool,"Could not create rows!"); goto complete; }
			// initialize the table for the number of columns
			table[rows] = apr_pcalloc(r->pool, cols * sizeof (oFields));
			if (table[rows] == NULL){ errors = apr_pstrdup(r->pool,"Could not create columns!"); goto complete; }
			// loop over columns
			for (col = 1; col <= cols; col++)
			{
				// retrieve the column name
				colName = apr_pstrdup(r->pool,(odbPSTR)odbColName( hQry, col ) );
				// retrieve column data type
				colType = odbColDataType( hQry, col );
				// try to retrieve the column
				if( odbColData( hQry, col ) ) 
				{
					// convert based on data type
					if (colType == ODB_CHAR || colType == ODB_WCHAR)
					{
						// set text data directly
						colData = apr_psprintf(r->pool, "%s", (odbPSTR)odbColData( hQry, col ) );
						// set data length
						colLen = odbColDataLen( hQry, col );
					}
					else if (colType == ODB_DATETIME)
					{
						// set text data directly
						odbTimestampToStr(colData,(odbPTIMESTAMP) odbColData( hQry, col ), 0);
						// set data length
						colLen = strlen(colData);
					}
					else if (colType == ODB_INT)
					{ 
						// set text data directly
						colData = apr_psprintf(r->pool, "%i", *((odbPLONG)odbColData( hQry, col ))); 
						// set data length
						colLen = strlen(colData);					
					}
					else {colData = apr_pstrdup(r->pool, "UNSUPPORTED"); colLen = 11;}
				} // end if
				else {colData = apr_pstrdup(r->pool, "NULL"); colLen = 4;} // null value detected
				// insert into table
				table[rows][col - 1].column = col;
				table[rows][col - 1].type = colType;
				table[rows][col - 1].name = apr_psprintf(r->pool, "%s", colName);
				table[rows][col - 1].data = apr_psprintf(r->pool, "%s", colData);
				table[rows][col - 1].length = colLen;
			} // end for
			// increment row count
			rows++;
		} // end while
	}
	// logout
	if( !odbLogout( hCon, FALSE ) ) { errors = apr_pstrdup(r->pool,"Could not logout"); goto complete; }
	// end
	goto complete;
	//---> Exit Procedure
	complete:
		// determine if the isError flag is set or there is any data in errors
		if (strlen(errors) || isError)
			isError = 1;
		else
			errors = apr_pstrdup(r->pool,"No Errors");
		/* set metadata members */
		metadata->isError = isError; // set bool isError flag
		metadata->rowcount = rows; // set row count
		metadata->colcount = cols; // set column count
		metadata->message = errors; // set errors
		/* clean up */
		odbFree( hCon ); // free odbtp connection
		odbWinsockCleanup(); // clean up network connection
		/* return */
		return table;
}

Open in new window

Infinity08

free should only be used when there's a corresponding malloc, calloc or realloc. I only see one of the latter functions being called in your code (realloc for table inside the oExecute function).

It seems that some of the memory is being allocated by apr_pcalloc, which allocates memory from an (already existing) pool. You don't free such allocated memory. Instead, you destroy the entire pool as soon as you don't need it any more.

http://apr.apache.org/docs/apr/0.9/group__apr__pools.html#gf61c098ad258069d64cdf8c0a9369f9e

Deepu Abraham

Have a look at this.

http://c-faq.com/~scs/cclass/int/sx9b.html

Also you could check for the memory before you free it.

// free memory
	for (row = 0; row < rows; row++){
	if(table[row])
		free(table[row]); // free columns
	}
	// free rows
	if(table)
		free(table);
	// free metadata
	if(metadata)
		free(metadata);

Open in new window

Infinity08

@DeepuAbrahamK : that would still causes problems, for the reasons I stated above.

Deepu Abraham

@Infinity08: My bad I didn't see your update before. I think you are spot on !

dilithiumtoys_dot_com

ASKER

@Infinity08: Should I call apr_pcalloc on the table[row] in a loop to set the memory to zero?

@DeepuAbrahamK: I can still use your solution to release table and metadata...so thanks!!!!

Infinity08

>> @Infinity08: Should I call apr_pcalloc on the table[row] in a loop to set the memory to zero?

You already are calling it in a loop.

>> @DeepuAbrahamK: I can still use your solution to release table and metadata...so thanks!!!!

metadata wasn't allocated with malloc, calloc or realloc, so you shouldn't use free to deallocate it.
table was allocated with realloc, but the free you had was already sufficient for that. adding an if (table) in front of it doesn't change anything, because the free function itself already performs that check.

dilithiumtoys_dot_com

ASKER

@infinity: My reference is to instead of this

for (row = 0; row < rows; row++)
free(table[row]); // free columns
THIS:
for (row = 0; row < rows; row++)
table[rows] = apr_pcalloc(r->pool, 0); // free columns

I want to explicitly destroy the memory and free it.

Are you saying that metadata does not need to be destroyed? I am new to programming in this enviornment, and I know Apache cleans up for itself, but I have no idea, to what extent.

Thanks

ASKER CERTIFIED SOLUTION

Infinity08

membership

Create a free account to see this answer

Signing up is free and takes 30 seconds. No credit card required.

See answer