Skip to content

Commit 5b4a1ec

Browse files
kocsismatekocsismate
committed
New reviww round
1 parent 4a7b5f6 commit 5b4a1ec

File tree

4 files changed

+55
-60
lines changed

4 files changed

+55
-60
lines changed

‎ext/odbc/odbc.stub.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,9 +373,9 @@ function odbc_result_all(ODBC\Result $statement, string $format = ""): int|false
373373

374374
function odbc_free_result(ODBC\Result $statement): true {}
375375

376-
function odbc_connect(string $dsn, ?string $user = null, #[\SensitiveParameter] string $password, int $cursor_option = SQL_CUR_USE_DRIVER): ODBC\Connection|false {}
376+
function odbc_connect(string $dsn, ?string $user = null, #[\SensitiveParameter] ?string $password = null, int $cursor_option = SQL_CUR_USE_DRIVER): ODBC\Connection|false {}
377377

378-
function odbc_pconnect(string $dsn, ?string $user = null, #[\SensitiveParameter] string $password, int $cursor_option = SQL_CUR_USE_DRIVER): ODBC\Connection|false {}
378+
function odbc_pconnect(string $dsn, ?string $user = null, #[\SensitiveParameter] ?string $password = null, int $cursor_option = SQL_CUR_USE_DRIVER): ODBC\Connection|false {}
379379

380380
function odbc_close(ODBC\Connection $odbc): void {}
381381

‎ext/odbc/odbc_arginfo.h

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/odbc/php_odbc.c

Lines changed: 49 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,10 @@ static zend_object_handlers odbc_connection_object_handlers, odbc_result_object_
7979

8080
static void odbc_insert_new_result(odbc_connection *connection, zval *result) {
8181
ZEND_ASSERT(Z_TYPE_P(result) == IS_OBJECT && instanceof_function(Z_OBJCE_P(result), odbc_result_ce));
82-
if (!connection->results) {
83-
connection->results = zend_new_array(1);
84-
}
8582

86-
zend_hash_next_index_insert_new(connection->results, result);
8783
odbc_result *res = Z_ODBC_RESULT_P(result);
88-
res->index = zend_hash_num_elements(connection->results) - 1;
84+
res->index = connection->results.nNextFreeElement;
85+
zend_hash_next_index_insert_new(&connection->results, result);
8986
Z_ADDREF_P(result);
9087
}
9188

@@ -107,13 +104,17 @@ static void odbc_link_free(odbc_link *link)
107104
efree(link->connection);
108105
ODBCG(num_links)--;
109106

110-
zend_hash_del(&ODBCG(non_persistent_connections), link->hash);
107+
if (link->hash) {
108+
zend_hash_del(&ODBCG(non_persistent_connections), link->hash);
109+
}
111110
}
112111

113112
link->connection = NULL;
114113

115-
zend_string_release(link->hash);
116-
link->hash = NULL;
114+
if (link->hash) {
115+
zend_string_release_ex(link->hash, link->persistent);
116+
link->hash = NULL;
117+
}
117118
}
118119

119120
static inline odbc_link *odbc_link_from_obj(zend_object *obj) {
@@ -197,11 +198,9 @@ static void odbc_result_free(odbc_result *res)
197198
res->param_info = NULL;
198199
}
199200

200-
HashTable *tmp = res->conn_ptr->results;
201+
HashTable *tmp = &res->conn_ptr->results;
201202
res->conn_ptr = NULL;
202-
if (tmp) {
203-
zend_hash_index_del(tmp, res->index);
204-
}
203+
zend_hash_index_del(tmp, res->index);
205204
}
206205

207206
static void odbc_result_free_obj(zend_object *obj) {
@@ -249,20 +248,14 @@ ZEND_GET_MODULE(odbc)
249248
static void close_results_with_connection(odbc_connection *conn) {
250249
zval *p;
251250

252-
if (conn->results == NULL) {
253-
return;
254-
}
255-
256-
ZEND_HASH_FOREACH_VAL(conn->results, p) {
251+
ZEND_HASH_FOREACH_VAL(&conn->results, p) {
257252
odbc_result *result = Z_ODBC_RESULT_P(p);
258253
if (result->conn_ptr) {
259254
odbc_result_free(result);
260255
}
261256
} ZEND_HASH_FOREACH_END();
262257

263-
zend_hash_destroy(conn->results);
264-
FREE_HASHTABLE(conn->results);
265-
conn->results = NULL;
258+
zend_hash_destroy(&conn->results);
266259
}
267260

268261
/* disconnect, and if it fails, then issue a rollback for any pending transaction (lurcher) */
@@ -852,14 +845,14 @@ PHP_FUNCTION(odbc_close_all)
852845
RETURN_THROWS();
853846
}
854847

848+
/* Loop through the non-persistent connection list, now close all non-persistent connections and their results */
855849
ZEND_HASH_FOREACH_VAL(&ODBCG(non_persistent_connections), zv) {
856850
odbc_link *link = Z_ODBC_LINK_P(zv);
857851
if (link->connection) {
858852
odbc_link_free(link);
859853
}
860854
} ZEND_HASH_FOREACH_END();
861855

862-
/* Loop through the non-persistent connection list, now close all non-persistent connections and their results */
863856
zend_hash_clean(&ODBCG(non_persistent_connections));
864857

865858
/* Loop through the persistent connection list, now close all persistent connections and their results */
@@ -1529,7 +1522,7 @@ PHP_FUNCTION(odbc_fetch_into)
15291522
SQLSMALLINT sql_c_type;
15301523
char *buf = NULL;
15311524
zval *pv_res, *pv_res_arr, tmp;
1532-
zend_long pv_row = -1;
1525+
zend_long pv_row = 0;
15331526
bool pv_row_is_null = true;
15341527
#ifdef HAVE_SQL_EXTENDED_FETCH
15351528
SQLULEN crow;
@@ -1658,7 +1651,7 @@ PHP_FUNCTION(odbc_fetch_row)
16581651
odbc_result *result;
16591652
RETCODE rc;
16601653
zval *pv_res;
1661-
zend_long pv_row = -1;
1654+
zend_long pv_row = 0;
16621655
bool pv_row_is_null = true;
16631656
#ifdef HAVE_SQL_EXTENDED_FETCH
16641657
SQLULEN crow;
@@ -2100,26 +2093,25 @@ PHP_FUNCTION(odbc_pconnect)
21002093
/* }}} */
21012094

21022095
/* {{{ odbc_sqlconnect */
2103-
bool odbc_sqlconnect(zval *zv, char *db, char *uid, char *pwd, int cur_opt, bool persistent, zend_string *hash)
2096+
bool odbc_sqlconnect(zval *zv, char *db, char *uid, char *pwd, int cur_opt, bool persistent, char *hash, int hash_len)
21042097
{
21052098
RETCODE rc;
21062099
SQLRETURN ret;
21072100
odbc_link *link;
21082101

21092102
object_init_ex(zv, odbc_connection_ce);
21102103
link = Z_ODBC_LINK_P(zv);
2111-
link->connection = (odbc_connection *)pemalloc(sizeof(odbc_connection), persistent);
2112-
memset(link->connection, 0, sizeof(odbc_connection));
2113-
2104+
link->connection = pecalloc(1, sizeof(odbc_connection), persistent);
2105+
zend_hash_init(&link->connection->results, 0, NULL, NULL, 1);
21142106
link->persistent = persistent;
2115-
link->hash = zend_string_copy(hash);
2116-
ret = SQLAllocEnv(&((*link->connection).henv));
2107+
link->hash = zend_string_init(hash, hash_len, persistent);
2108+
ret = SQLAllocEnv(&link->connection->henv);
21172109
if (ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO) {
21182110
odbc_sql_error(link->connection, SQL_NULL_HSTMT, "SQLAllocEnv");
21192111
return false;
21202112
}
21212113

2122-
ret = SQLAllocConnect((*link->connection).henv, &((*link->connection).hdbc));
2114+
ret = SQLAllocConnect(link->connection->henv, &((*link->connection).hdbc));
21232115
if (ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO) {
21242116
odbc_sql_error(link->connection, SQL_NULL_HSTMT, "SQLAllocConnect");
21252117
return false;
@@ -2244,11 +2236,6 @@ bool odbc_sqlconnect(zval *zv, char *db, char *uid, char *pwd, int cur_opt, bool
22442236
}
22452237
/* }}} */
22462238

2247-
/* Persistent connections: two list-types le_pconn, le_conn and a plist
2248-
* where hashed connection info is stored together with index pointer to
2249-
* the actual link of type le_pconn in the list. Only persistent
2250-
* connections get hashed up.
2251-
*/
22522239
/* {{{ odbc_do_connect */
22532240
void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
22542241
{
@@ -2284,37 +2271,39 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
22842271
persistent = 0;
22852272
}
22862273

2287-
smart_str hashed_details = {0};
2288-
smart_str_append_printf(&hashed_details, "%s_%s_%s_%s_%d", ODBC_TYPE, db, uid, pwd, cur_opt);
2274+
char *hashed_details;
2275+
int hashed_details_len;
2276+
2277+
hashed_details_len = spprintf(&hashed_details, 0, "%s_%s_%s_%s_%d", ODBC_TYPE, db, uid, pwd, cur_opt);
22892278

22902279
try_and_get_another_connection:
22912280

22922281
if (persistent) {
22932282
zend_resource *le;
22942283

22952284
/* the link is not in the persistent list */
2296-
if ((le = zend_hash_find_ptr(&EG(persistent_list), hashed_details.s)) == NULL) {
2285+
if ((le = zend_hash_str_find_ptr(&EG(persistent_list), hashed_details, hashed_details_len)) == NULL) {
22972286
if (ODBCG(max_links) != -1 && ODBCG(num_links) >= ODBCG(max_links)) {
22982287
php_error_docref(NULL, E_WARNING, "Too many open links (" ZEND_LONG_FMT ")", ODBCG(num_links));
2299-
smart_str_free(&hashed_details);
2288+
efree(hashed_details);
23002289
RETURN_FALSE;
23012290
}
23022291
if (ODBCG(max_persistent) != -1 && ODBCG(num_persistent) >= ODBCG(max_persistent)) {
23032292
php_error_docref(NULL, E_WARNING,"Too many open persistent links (" ZEND_LONG_FMT ")", ODBCG(num_persistent));
2304-
smart_str_free(&hashed_details);
2293+
efree(hashed_details);
23052294
RETURN_FALSE;
23062295
}
23072296

2308-
if (!odbc_sqlconnect(return_value, db, uid, pwd, cur_opt, true, hashed_details.s)) {
2309-
smart_str_free(&hashed_details);
2297+
if (!odbc_sqlconnect(return_value, db, uid, pwd, cur_opt, true, hashed_details, hashed_details_len)) {
2298+
efree(hashed_details);
23102299
zval_ptr_dtor(return_value);
23112300
RETURN_FALSE;
23122301
}
23132302

23142303
db_conn = Z_ODBC_CONNECTION_P(return_value);
23152304

2316-
if (zend_register_persistent_resource(ZSTR_VAL(hashed_details.s), ZSTR_LEN(hashed_details.s), db_conn, le_pconn) == NULL) {
2317-
smart_str_free(&hashed_details);
2305+
if (zend_register_persistent_resource(hashed_details, hashed_details_len, db_conn, le_pconn) == NULL) {
2306+
efree(hashed_details);
23182307
zval_ptr_dtor(return_value);
23192308
RETURN_FALSE;
23202309
}
@@ -2343,7 +2332,7 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
23432332
&dead, 0, NULL);
23442333
if (ret == SQL_SUCCESS && dead == SQL_CD_TRUE) {
23452334
/* Bail early here, since we know it's gone */
2346-
zend_hash_del(&EG(persistent_list), hashed_details.s);
2335+
zend_hash_str_del(&EG(persistent_list), hashed_details, hashed_details_len);
23472336
goto try_and_get_another_connection;
23482337
}
23492338
/* If the driver doesn't support it, or returns
@@ -2355,7 +2344,7 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
23552344
d_name, sizeof(d_name), &len);
23562345

23572346
if(ret != SQL_SUCCESS || len == 0) {
2358-
zend_hash_del(&EG(persistent_list), hashed_details.s);
2347+
zend_hash_str_del(&EG(persistent_list), hashed_details, hashed_details_len);
23592348
/* Commented out to fix a possible double closure error
23602349
* when working with persistent connections as submitted by
23612350
* bug #15758
@@ -2370,38 +2359,38 @@ void odbc_do_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
23702359
object_init_ex(return_value, odbc_connection_ce);
23712360
odbc_link *link = Z_ODBC_LINK_P(return_value);
23722361
link->connection = db_conn;
2373-
link->hash = zend_string_copy(hashed_details.s);
2362+
link->hash = zend_string_init(hashed_details, hashed_details_len, persistent);
23742363
link->persistent = true;
23752364
}
23762365
} else {
23772366
zval *tmp;
2378-
if ((tmp = zend_hash_find(&ODBCG(non_persistent_connections), hashed_details.s)) == NULL) { /* non-persistent, new */
2367+
if ((tmp = zend_hash_str_find(&ODBCG(non_persistent_connections), hashed_details, hashed_details_len)) == NULL) { /* non-persistent, new */
23792368
if (ODBCG(max_links) != -1 && ODBCG(num_links) >= ODBCG(max_links)) {
23802369
php_error_docref(NULL, E_WARNING, "Too many open connections (" ZEND_LONG_FMT ")", ODBCG(num_links));
2381-
smart_str_free(&hashed_details);
2370+
efree(hashed_details);
23822371
RETURN_FALSE;
23832372
}
23842373

2385-
if (!odbc_sqlconnect(return_value, db, uid, pwd, cur_opt, false, hashed_details.s)) {
2386-
smart_str_free(&hashed_details);
2374+
if (!odbc_sqlconnect(return_value, db, uid, pwd, cur_opt, false, hashed_details, hashed_details_len)) {
2375+
efree(hashed_details);
23872376
zval_ptr_dtor(return_value);
23882377
RETURN_FALSE;
23892378
}
23902379
ODBCG(num_links)++;
23912380

2392-
zend_hash_add_new(&ODBCG(non_persistent_connections), hashed_details.s, return_value);
2381+
zend_hash_str_add_new(&ODBCG(non_persistent_connections), hashed_details, hashed_details_len, return_value);
23932382
} else { /* non-persistent, pre-existing */
23942383
ZVAL_COPY(return_value, tmp);
23952384
}
23962385
}
2397-
smart_str_free(&hashed_details);
2386+
efree(hashed_details);
23982387
}
23992388
/* }}} */
24002389

24012390
/* {{{ Close an ODBC connection */
24022391
PHP_FUNCTION(odbc_close)
24032392
{
2404-
zval *pv_conn;
2393+
zval *pv_conn, *zv;
24052394
odbc_link *link;
24062395

24072396
if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &pv_conn, odbc_connection_ce) == FAILURE) {
@@ -2412,6 +2401,12 @@ PHP_FUNCTION(odbc_close)
24122401
CHECK_ODBC_CONNECTION(link->connection);
24132402

24142403
odbc_link_free(link);
2404+
2405+
ZEND_HASH_FOREACH_VAL(&EG(persistent_list), zv) {
2406+
if (Z_RES_P(zv)->type == le_pconn && link->connection == Z_RES_P(zv)->ptr) {
2407+
zend_list_close(Z_RES_P(zv));
2408+
}
2409+
} ZEND_HASH_FOREACH_END();
24152410
}
24162411
/* }}} */
24172412

‎ext/odbc/php_odbc_includes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ typedef struct odbc_connection {
191191
ODBC_SQL_CONN_T hdbc;
192192
char laststate[6];
193193
char lasterrormsg[SQL_MAX_MESSAGE_LENGTH];
194-
HashTable *results;
194+
HashTable results;
195195
} odbc_connection;
196196

197197
typedef struct odbc_link {

0 commit comments

Comments
 (0)