#30280 [Asn->Csd]: [PATCH] memleaks and other minor issues

ID: 30280 Updated by: fmk@php.net Reported By: freddyz77 at tin dot it -Status: Assigned +Status: Closed Bug Type: MSSQL related Operating System: Any PHP Version: 4.3.9 Assigned To: fmk New Comment: This bug has been fixed in CVS. Snapshots of the sources are packaged every three hours; this change will be in the next snapshot. You can grab the snapshot at http://snaps.php.net/. Thank you for the report, and for helping us make PHP better. Most of this patch makes good sence and have been added to CVS. Thanks for the input. Previous Comments: ------------------------------------------------------------------------ [2004-09-30 12:24:44] andreyra at chtivo dot ru How to get the compiled version of this dll for Windows? ------------------------------------------------------------------------ [2004-09-29 20:39:37] freddyz77 at tin dot it Description: ------------ Well, I wrote this patch just looking at the code, I hope someone find some useful info. Mainly it contains some memory leak fix connecting and some others minor issues and notes. --- php_mssql.c.orig 2004-09-29 19:52:42.000000000 +0200 +++ php_mssql.c 2004-09-29 20:29:10.000000000 +0200 @@ -197,20 +197,23 @@ result->data = NULL; result->blocks_initialized = 0; } if (free_fields && result->fields) { for (i=0; i<result->num_fields; i++) { STR_FREE(result->fields[i].name); STR_FREE(result->fields[i].column_source); } efree(result->fields); + result->fields = NULL; + result->num_fields = 0; + result->cur_field = 0; } } static void _free_mssql_statement(mssql_statement *statement) { if (statement->binds) { zend_hash_destroy(statement->binds); efree(statement->binds); } @@ -263,28 +266,25 @@ mssql_bind *bind= (mssql_bind *) data; zval_ptr_dtor(&(bind->zval)); } static void php_mssql_init_globals(zend_mssql_globals *mssql_globals) { long compatability_mode; mssql_globals->num_persistent = 0; + mssql_globals->get_column_content = php_mssql_get_column_content_with_type; if (cfg_get_long("mssql.compatability_mode", &compatability_mode) == SUCCESS) { if (compatability_mode) { mssql_globals->get_column_content = php_mssql_get_column_content_without_type; - } else { - mssql_globals->get_column_content = php_mssql_get_column_content_with_type; } - } else { - mssql_globals->get_column_content = php_mssql_get_column_content_with_type; } } PHP_MINIT_FUNCTION(mssql) { ZEND_INIT_MODULE_GLOBALS(mssql, php_mssql_init_globals, NULL); REGISTER_INI_ENTRIES(); le_statement = register_list_destructors(_free_mssql_statement, NULL); @@ -508,61 +508,68 @@ if (DBSETOPT(mssql.link, DBBUFFER, "2")==FAIL) { efree(hashed_details); dbfreelogin(mssql.login); dbclose(mssql.link); RETURN_FALSE; } if (MS_SQL_G(textlimit) != -1) { sprintf(buffer, "%li", MS_SQL_G(textlimit)); if (DBSETOPT(mssql.link, DBTEXTLIMIT, buffer)==FAIL) { + dbclose(mssql.link); efree(hashed_details); dbfreelogin(mssql.login); RETURN_FALSE; } } if (MS_SQL_G(textsize) != -1) { sprintf(buffer, "SET TEXTSIZE %li", MS_SQL_G(textsize)); dbcmd(mssql.link, buffer); dbsqlexec(mssql.link); dbresults(mssql.link); } /* hash it up */ mssql_ptr = (mssql_link *) malloc(sizeof(mssql_link)); memcpy(mssql_ptr, &mssql, sizeof(mssql_link)); Z_TYPE(new_le) = le_plink; new_le.ptr = mssql_ptr; if (zend_hash_update(&EG(persistent_list), hashed_details, hashed_details_length + 1, &new_le, sizeof(list_entry), NULL)==FAILURE) { free(mssql_ptr); + dbclose(mssql.link); efree(hashed_details); dbfreelogin(mssql.login); RETURN_FALSE; } MS_SQL_G(num_persistent)++; MS_SQL_G(num_links)++; } else { /* we do */ + dbfreelogin(mssql.login); + if (Z_TYPE_P(le) != le_plink) { + efree(hashed_details); #if BROKEN_MSSQL_PCONNECTS log_error("PHP/MS SQL: Hashed persistent link is not a MS SQL link!",php_rqst->server); #endif php_error_docref(NULL TSRMLS_CC, E_WARNING, "Hashed persistent link is not a MS SQL link!"); RETURN_FALSE; } mssql_ptr = (mssql_link *) le->ptr; /* test that the link hasn't died */ if (DBDEAD(mssql_ptr->link) == TRUE) { + /* free previous connection */ + dbclose(mssql_ptr->link); #if BROKEN_MSSQL_PCONNECTS log_error("PHP/MS SQL: Persistent link died, trying to reconnect...",php_rqst->server); #endif - if ((mssql_ptr->link=dbopen(mssql_ptr->login,host))==FAIL) { + if ((mssql_ptr->link=dbopen(mssql_ptr->login,host))==NULL) { #if BROKEN_MSSQL_PCONNECTS log_error("PHP/MS SQL: Unable to reconnect!",php_rqst->server); #endif php_error_docref(NULL TSRMLS_CC, E_WARNING, "Link to server lost, unable to reconnect"); zend_hash_del(&EG(persistent_list), hashed_details, hashed_details_length+1); efree(hashed_details); RETURN_FALSE; } #if BROKEN_MSSQL_PCONNECTS log_error("PHP/MS SQL: Reconnect successful!",php_rqst->server); @@ -584,57 +591,63 @@ /* first we check the hash for the hashed_details key. if it exists, * it should point us to the right offset where the actual mssql link sits. * if it doesn't, open a new mssql link, add it to the resource list, * and add a pointer to it with hashed_details as the key. */ if (zend_hash_find(&EG(regular_list), hashed_details, hashed_details_length + 1,(void **) &index_ptr)==SUCCESS) { int type,link; void *ptr; if (Z_TYPE_P(index_ptr) != le_index_ptr) { + efree(hashed_details); + dbfreelogin(mssql.login); RETURN_FALSE; } link = (int) index_ptr->ptr; ptr = zend_list_find(link,&type); /* check if the link is still there */ if (ptr && (type==le_link || type==le_plink)) { zend_list_addref(link); Z_LVAL_P(return_value) = link; php_mssql_set_default_link(link TSRMLS_CC); Z_TYPE_P(return_value) = IS_RESOURCE; efree(hashed_details); + dbfreelogin(mssql.login); return; } else { zend_hash_del(&EG(regular_list), hashed_details, hashed_details_length + 1); } } if (MS_SQL_G(max_links) != -1 && MS_SQL_G(num_links) >= MS_SQL_G(max_links)) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Too many open links (%ld)", MS_SQL_G(num_links)); efree(hashed_details); + dbfreelogin(mssql.login); RETURN_FALSE; } if ((mssql.link=dbopen(mssql.login, host))==NULL) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unable to connect to server: %s", host); efree(hashed_details); + dbfreelogin(mssql.login); RETURN_FALSE; } if (DBSETOPT(mssql.link, DBBUFFER,"2")==FAIL) { efree(hashed_details); dbfreelogin(mssql.login); dbclose(mssql.link); RETURN_FALSE; } if (MS_SQL_G(textlimit) != -1) { sprintf(buffer, "%li", MS_SQL_G(textlimit)); if (DBSETOPT(mssql.link, DBTEXTLIMIT, buffer)==FAIL) { + dbclose(mssql.link); efree(hashed_details); dbfreelogin(mssql.login); RETURN_FALSE; } } if (MS_SQL_G(textsize) != -1) { sprintf(buffer, "SET TEXTSIZE %li", MS_SQL_G(textsize)); dbcmd(mssql.link, buffer); dbsqlexec(mssql.link); dbresults(mssql.link); @@ -980,40 +993,42 @@ type = dbrettype(mssql_ptr->link, i); if (statement->binds != NULL) { /* Maybe a non-parameter sp */ if (zend_hash_find(statement->binds, parameter, strlen(parameter), (void**)&bind)==SUCCESS) { switch (type) { case SQLBIT: case SQLINT1: case SQLINT2: case SQLINT4: convert_to_long_ex(&bind->zval); + /* FIXME this works only on little endian machine !!! */ Z_LVAL_P(bind->zval) = *((int *)(dbretdata(mssql_ptr->link,i))); break; case SQLFLT4: case SQLFLT8: case SQLFLTN: case SQLMONEY4: case SQLMONEY: case SQLMONEYN: convert_to_double_ex(&bind->zval); Z_DVAL_P(bind->zval) = *((double *)(dbretdata(mssql_ptr->link,i))); break; case SQLCHAR: case SQLVARCHAR: case SQLTEXT: convert_to_string_ex(&bind->zval); Z_STRLEN_P(bind->zval) = dbretlen(mssql_ptr->link,i); Z_STRVAL_P(bind->zval) = estrndup(dbretdata(mssql_ptr->link,i),Z_STRLEN_P(bind->zval)); break; + /* TODO binary */ } } else { php_error_docref(NULL TSRMLS_CC, E_WARNING, "An output parameter variable was not provided"); } } } } if (statement->binds != NULL) { /* Maybe a non-parameter sp */ if (zend_hash_find(statement->binds, "RETVAL", 6, (void**)&bind)==SUCCESS) { @@ -1187,40 +1202,44 @@ } if (dbsqlexec(mssql_ptr->link)==FAIL || (retvalue = dbresults(mssql_ptr->link))==FAIL) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Query failed"); RETURN_FALSE; } /* Skip results not returning any columns */ while ((num_fields = dbnumcols(mssql_ptr->link)) <= 0 && retvalue == SUCCEED) { retvalue = dbresults(mssql_ptr->link); } + if (retvalue != SUCCEED) { + RETURN_FALSE; + } if ((num_fields = dbnumcols(mssql_ptr->link)) <= 0) { RETURN_TRUE; } retvalue=dbnextrow(mssql_ptr->link); if (retvalue==FAIL) { RETURN_FALSE; } result = (mssql_result *) emalloc(sizeof(mssql_result)); result->statement = NULL; result->num_fields = num_fields; result->blocks_initialized = 1; result->batchsize = batchsize; result->data = NULL; result->blocks_initialized = 0; result->mssql_ptr = mssql_ptr; result->cur_field=result->cur_row=result->num_rows=0; + /* TODO here this condition it's always true */ if (num_fields > 0) { result->fields = (mssql_field *) emalloc(sizeof(mssql_field)*result->num_fields); result->num_rows = _mssql_fetch_batch(mssql_ptr, result, retvalue TSRMLS_CC); } else result->fields = NULL; ZEND_REGISTER_RESOURCE(return_value, result, le_result); } /* }}} */ @@ -1267,20 +1286,21 @@ zend_list_delete(Z_RESVAL_PP(mssql_result_index)); RETURN_TRUE; } /* }}} */ /* {{{ proto string mssql_get_last_message(void) Gets the last message from the MS-SQL server */ PHP_FUNCTION(mssql_get_last_message) { + /* TODO add link parameter */ if (MS_SQL_G(server_message)) { RETURN_STRING(MS_SQL_G(server_message),1); } else { RETURN_STRING(empty_string,1); } } /* }}} */ @@ -2236,27 +2256,29 @@ convert_to_string_ex(binary); convert_to_long_ex(short_format); sf = Z_LVAL_PP(short_format); break; default: WRONG_PARAM_COUNT; break; } + /* FIXME this code can cause a buffer reading overflow if binary it's less than 16 bytes */ dbconvert(NULL, SQLBINARY, (BYTE*)Z_STRVAL_PP(binary), 16, SQLCHAR, buffer, -1); if (sf) { php_strtoupper(buffer, 32); RETURN_STRING(buffer, 1); } else { + /* FIXME this works only on little endian machine */ int i; for (i=0; i<4; i++) { buffer2[2*i] = buffer[6-2*i]; buffer2[2*i+1] = buffer[7-2*i]; } buffer2[8] = '-'; for (i=0; i<2; i++) { buffer2[9+2*i] = buffer[10-2*i]; buffer2[10+2*i] = buffer[11-2*i]; } ------------------------------------------------------------------------ -- Edit this bug report at http://bugs.php.net/?id=30280&edit=1