Skip to content

Commit 49d3f2a

Browse files
matyhtfmatyhtf
authored
optimize code (swoole#4076)
1 parent 663156d commit 49d3f2a

File tree

7 files changed

+52
-66
lines changed

7 files changed

+52
-66
lines changed

‎ext-src/swoole_client_coro.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ static PHP_METHOD(swoole_client_coro, peek) {
789789

790790
static PHP_METHOD(swoole_client_coro, isConnected) {
791791
Socket *cli = php_swoole_get_sock(ZEND_THIS);
792-
if (cli && cli->is_connect()) {
792+
if (cli && cli->is_connected()) {
793793
RETURN_TRUE;
794794
} else {
795795
RETURN_FALSE;

‎ext-src/swoole_http2_client_coro.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ class Client {
119119
}
120120

121121
inline bool is_available() {
122-
if (sw_unlikely(!client || !client->is_connect())) {
122+
if (sw_unlikely(!client || !client->is_connected())) {
123123
swoole_set_last_error(SW_ERROR_CLIENT_NO_CONNECTION);
124124
zend_update_property_long(
125125
swoole_http2_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), SW_ERROR_CLIENT_NO_CONNECTION);

‎ext-src/swoole_http_client_coro.cc

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ class HttpClient {
141141
#endif
142142
bool bind(std::string address, int port = 0);
143143
bool connect();
144+
void set_error(int error, const char *msg, int status);
144145
bool keep_liveness();
145146
bool send();
146147
void reset();
@@ -174,7 +175,7 @@ class HttpClient {
174175
#endif
175176
void apply_setting(zval *zset, const bool check_all = true);
176177
void set_basic_auth(const std::string &username, const std::string &password);
177-
bool exec(std::string path);
178+
bool exec(std::string _path);
178179
bool recv(double timeout = 0);
179180
void recv(zval *zframe, double timeout = 0);
180181
bool recv_http_response(double timeout = 0);
@@ -808,14 +809,7 @@ bool HttpClient::connect() {
808809
if (!body) {
809810
body = new String(SW_HTTP_RESPONSE_INIT_SIZE);
810811
if (!body) {
811-
zend_update_property_long(
812-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), ENOMEM);
813-
zend_update_property_string(
814-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errMsg"), swoole_strerror(ENOMEM));
815-
zend_update_property_long(swoole_http_client_coro_ce,
816-
SW_Z8_OBJ_P(zobject),
817-
ZEND_STRL("statusCode"),
818-
HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
812+
set_error(ENOMEM, swoole_strerror(ENOMEM), HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
819813
return false;
820814
}
821815
}
@@ -824,13 +818,7 @@ bool HttpClient::connect() {
824818
socket = new Socket(socket_type);
825819
if (UNEXPECTED(socket->get_fd() < 0)) {
826820
php_swoole_sys_error(E_WARNING, "new Socket() failed");
827-
zend_update_property_long(swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), errno);
828-
zend_update_property_string(
829-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errMsg"), swoole_strerror(errno));
830-
zend_update_property_long(swoole_http_client_coro_ce,
831-
SW_Z8_OBJ_P(zobject),
832-
ZEND_STRL("statusCode"),
833-
HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
821+
set_error(errno, swoole_strerror(errno), HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
834822
delete socket;
835823
socket = nullptr;
836824
return false;
@@ -849,14 +837,7 @@ bool HttpClient::connect() {
849837
// connect
850838
socket->set_timeout(connect_timeout, Socket::TIMEOUT_CONNECT);
851839
if (!socket->connect(host, port)) {
852-
zend_update_property_long(
853-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), socket->errCode);
854-
zend_update_property_string(
855-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errMsg"), socket->errMsg);
856-
zend_update_property_long(swoole_http_client_coro_ce,
857-
SW_Z8_OBJ_P(zobject),
858-
ZEND_STRL("statusCode"),
859-
HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
840+
set_error(socket->errCode, socket->errMsg, HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
860841
close();
861842
return false;
862843
}
@@ -866,19 +847,20 @@ bool HttpClient::connect() {
866847
return true;
867848
}
868849

850+
void HttpClient::set_error(int error, const char *msg, int status) {
851+
auto ce = swoole_http_client_coro_ce;
852+
auto obj = SW_Z8_OBJ_P(zobject);
853+
zend_update_property_long(ce, obj, ZEND_STRL("errCode"), error);
854+
zend_update_property_string(ce, obj, ZEND_STRL("errMsg"), msg);
855+
zend_update_property_long(ce, obj, ZEND_STRL("statusCode"), status);
856+
}
857+
869858
bool HttpClient::keep_liveness() {
870859
if (!socket || !socket->check_liveness()) {
871860
if (socket) {
872861
/* in progress */
873862
socket->check_bound_co(SW_EVENT_RDWR);
874-
zend_update_property_long(
875-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), socket->errCode);
876-
zend_update_property_string(
877-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errMsg"), socket->errMsg);
878-
zend_update_property_long(swoole_http_client_coro_ce,
879-
SW_Z8_OBJ_P(zobject),
880-
ZEND_STRL("statusCode"),
881-
HTTP_CLIENT_ESTATUS_SERVER_RESET);
863+
set_error(socket->errCode, socket->errMsg, HTTP_CLIENT_ESTATUS_SERVER_RESET);
882864
close(false);
883865
}
884866
for (; reconnected_count < reconnect_interval; reconnected_count++) {
@@ -1350,21 +1332,16 @@ bool HttpClient::send() {
13501332

13511333
if (socket->send_all(buffer->str, buffer->length) != (ssize_t) buffer->length) {
13521334
_send_fail:
1353-
zend_update_property_long(
1354-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), socket->errCode);
1355-
zend_update_property_string(
1356-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errMsg"), socket->errMsg);
1357-
zend_update_property_long(
1358-
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("statusCode"), HTTP_CLIENT_ESTATUS_SEND_FAILED);
1335+
set_error(socket->errCode, socket->errMsg, HTTP_CLIENT_ESTATUS_SEND_FAILED);
13591336
close();
13601337
return false;
13611338
}
13621339
wait = true;
13631340
return true;
13641341
}
13651342

1366-
bool HttpClient::exec(std::string path) {
1367-
this->path = path;
1343+
bool HttpClient::exec(std::string _path) {
1344+
path = _path;
13681345
// bzero when make a new reqeust
13691346
reconnected_count = 0;
13701347
if (defer) {
@@ -1378,7 +1355,7 @@ bool HttpClient::recv(double timeout) {
13781355
if (!wait) {
13791356
return false;
13801357
}
1381-
if (!socket || !socket->is_connect()) {
1358+
if (!socket || !socket->is_connected()) {
13821359
swoole_set_last_error(SW_ERROR_CLIENT_NO_CONNECTION);
13831360
zend_update_property_long(
13841361
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), swoole_get_last_error());
@@ -1426,7 +1403,7 @@ bool HttpClient::recv(double timeout) {
14261403
void HttpClient::recv(zval *zframe, double timeout) {
14271404
SW_ASSERT(websocket);
14281405
ZVAL_FALSE(zframe);
1429-
if (!socket || !socket->is_connect()) {
1406+
if (!socket || !socket->is_connected()) {
14301407
swoole_set_last_error(SW_ERROR_CLIENT_NO_CONNECTION);
14311408
zend_update_property_long(
14321409
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), swoole_get_last_error());
@@ -1580,7 +1557,7 @@ bool HttpClient::push(zval *zdata, zend_long opcode, uint8_t flags) {
15801557
HTTP_CLIENT_ESTATUS_CONNECT_FAILED);
15811558
return false;
15821559
}
1583-
if (!socket || !socket->is_connect()) {
1560+
if (!socket || !socket->is_connected()) {
15841561
swoole_set_last_error(SW_ERROR_CLIENT_NO_CONNECTION);
15851562
zend_update_property_long(
15861563
swoole_http_client_coro_ce, SW_Z8_OBJ_P(zobject), ZEND_STRL("errCode"), swoole_get_last_error());
@@ -1779,7 +1756,7 @@ static PHP_METHOD(swoole_http_client_coro, __construct) {
17791756
HttpClientObject *hcc = php_swoole_http_client_coro_fetch_object(Z_OBJ_P(ZEND_THIS));
17801757
char *host;
17811758
size_t host_len;
1782-
zend_long port = 80;
1759+
zend_long port = 0;
17831760
zend_bool ssl = 0;
17841761

17851762
ZEND_PARSE_PARAMETERS_START_EX(ZEND_PARSE_PARAMS_THROW, 1, 3)
@@ -1807,6 +1784,9 @@ static PHP_METHOD(swoole_http_client_coro, __construct) {
18071784
RETURN_FALSE;
18081785
}
18091786
#endif
1787+
if (port == 0) {
1788+
port = ssl ? 443 : 80;
1789+
}
18101790
hcc->phc = new HttpClient(ZEND_THIS, std::string(host, host_len), port, ssl);
18111791
}
18121792

‎ext-src/swoole_mysql_coro.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,16 +191,16 @@ class mysql_client {
191191
return connect(host, port, ssl);
192192
}
193193

194-
inline bool is_connect() {
195-
return socket && socket->is_connect();
194+
inline bool is_connected() {
195+
return socket && socket->is_connected();
196196
}
197197

198198
inline int get_fd() {
199199
return socket ? socket->get_fd() : -1;
200200
}
201201

202202
inline bool check_connection() {
203-
if (sw_unlikely(!is_connect())) {
203+
if (sw_unlikely(!is_connected())) {
204204
non_sql_error(MYSQLND_CR_CONNECTION_ERROR, "%s or %s", strerror(ECONNRESET), strerror(ENOTCONN));
205205
return false;
206206
}
@@ -220,7 +220,7 @@ class mysql_client {
220220
}
221221

222222
inline bool is_writable() {
223-
return is_connect() && !socket->has_bound(SW_EVENT_WRITE);
223+
return is_connected() && !socket->has_bound(SW_EVENT_WRITE);
224224
}
225225

226226
bool is_available_for_new_request() {
@@ -1011,7 +1011,7 @@ void mysql_client::handle_strict_type(zval *ztext, mysql::field_packet *field) {
10111011
}
10121012

10131013
void mysql_client::fetch(zval *return_value) {
1014-
if (sw_unlikely(!is_connect())) {
1014+
if (sw_unlikely(!is_connected())) {
10151015
RETURN_FALSE;
10161016
}
10171017
if (sw_unlikely(state != SW_MYSQL_STATE_QUERY_FETCH)) {
@@ -1881,7 +1881,7 @@ static PHP_METHOD(swoole_mysql_coro, fetch) {
18811881
mc->fetch(return_value);
18821882
mc->del_timeout_controller();
18831883
if (sw_unlikely(Z_TYPE_P(return_value) == IS_FALSE)) {
1884-
swoole_mysql_coro_sync_error_properties(ZEND_THIS, mc->get_error_code(), mc->get_error_msg(), mc->is_connect());
1884+
swoole_mysql_coro_sync_error_properties(ZEND_THIS, mc->get_error_code(), mc->get_error_msg(), mc->is_connected());
18851885
}
18861886
}
18871887

@@ -1898,7 +1898,7 @@ static PHP_METHOD(swoole_mysql_coro, fetchAll) {
18981898
mc->fetch_all(return_value);
18991899
mc->del_timeout_controller();
19001900
if (sw_unlikely(Z_TYPE_P(return_value) == IS_FALSE)) {
1901-
swoole_mysql_coro_sync_error_properties(ZEND_THIS, mc->get_error_code(), mc->get_error_msg(), mc->is_connect());
1901+
swoole_mysql_coro_sync_error_properties(ZEND_THIS, mc->get_error_code(), mc->get_error_msg(), mc->is_connected());
19021902
}
19031903
}
19041904

@@ -1938,7 +1938,7 @@ static PHP_METHOD(swoole_mysql_coro, prepare) {
19381938
mc->add_timeout_controller(timeout, Socket::TIMEOUT_RDWR);
19391939
if (UNEXPECTED(!mc->send_prepare_request(statement, statement_length))) {
19401940
_failed:
1941-
swoole_mysql_coro_sync_error_properties(ZEND_THIS, mc->get_error_code(), mc->get_error_msg(), mc->is_connect());
1941+
swoole_mysql_coro_sync_error_properties(ZEND_THIS, mc->get_error_code(), mc->get_error_msg(), mc->is_connected());
19421942
RETVAL_FALSE;
19431943
} else if (UNEXPECTED(mc->get_defer())) {
19441944
RETVAL_TRUE;

‎include/swoole_coroutine_socket.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class Socket {
6969
bool cancel(const enum swEvent_type event);
7070
bool close();
7171

72-
inline bool is_connect() {
72+
inline bool is_connected() {
7373
return connected && !closed;
7474
}
7575

@@ -289,9 +289,11 @@ class Socket {
289289
return connect_timeout;
290290
} else if (type == TIMEOUT_READ) {
291291
return read_timeout;
292-
} else // if (type == TIMEOUT_WRITE)
293-
{
292+
} else if (type == TIMEOUT_WRITE) {
294293
return write_timeout;
294+
} else {
295+
assert(0);
296+
return -1;
295297
}
296298
}
297299

@@ -480,10 +482,8 @@ class Socket {
480482
if (timeout > 0) {
481483
*timer_pp = swoole_timer_add((long) (timeout * 1000), false, callback, socket_);
482484
return *timer_pp != nullptr;
483-
} else // if (timeout < 0)
484-
{
485-
*timer_pp = (TimerNode *) -1;
486485
}
486+
*timer_pp = (TimerNode *) -1;
487487
}
488488
return true;
489489
}

‎src/coroutine/socket.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ bool Socket::add_event(const enum swEvent_type event) {
124124
bool Socket::wait_event(const enum swEvent_type event, const void **__buf, size_t __n) {
125125
enum swEvent_type added_event = event;
126126
Coroutine *co = Coroutine::get_current_safe();
127+
if (!co) {
128+
return false;
129+
}
127130

128131
// clear the last errCode
129132
set_err(0);
@@ -156,8 +159,7 @@ bool Socket::wait_event(const enum swEvent_type event, const void **__buf, size_
156159
read_co = co;
157160
read_co->yield();
158161
read_co = nullptr;
159-
} else // if (event == SW_EVENT_WRITE)
160-
{
162+
} else if (event == SW_EVENT_WRITE) {
161163
if (sw_unlikely(!zero_copy && __n > 0 && *__buf != get_write_buffer()->str)) {
162164
write_buffer->clear();
163165
if (write_buffer->append((const char *) *__buf, __n) != SW_OK) {
@@ -169,6 +171,9 @@ bool Socket::wait_event(const enum swEvent_type event, const void **__buf, size_
169171
write_co = co;
170172
write_co->yield();
171173
write_co = nullptr;
174+
} else {
175+
assert(0);
176+
return false;
172177
}
173178
_failed:
174179
#ifdef SW_USE_OPENSSL
@@ -709,6 +714,7 @@ bool Socket::connect(std::string _host, int _port, int flags) {
709714
if (connect(_target_addr, socket->info.len) == false) {
710715
return false;
711716
}
717+
712718
// socks5 proxy
713719
if (socks5_proxy && socks5_handshake() == false) {
714720
if (errCode == 0) {
@@ -1612,7 +1618,7 @@ ssize_t Socket::recv_packet(double timeout) {
16121618

16131619
bool Socket::shutdown(int __how) {
16141620
set_err(0);
1615-
if (!is_connect() || (__how == SHUT_RD && shutdown_read) || (__how == SHUT_WR && shutdown_write)) {
1621+
if (!is_connected() || (__how == SHUT_RD && shutdown_read) || (__how == SHUT_WR && shutdown_write)) {
16161622
errno = ENOTCONN;
16171623
} else {
16181624
#ifdef SW_USE_OPENSSL

‎tests/swoole_client_sync/sendto.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ $pm = new SwooleTest\ProcessManager;
1515
$pm->parentFunc = function () use ($pm) {
1616
$cli = new Client(SWOOLE_SOCK_UDP);
1717

18-
$cli->sendto('127.0.0.1', $pm->getFreePort(), "packet-1");
19-
$cli->sendto('localhost', $pm->getFreePort(), "packet-2");
18+
Assert::true($cli->sendto('127.0.0.1', $pm->getFreePort(), "packet-1"));
19+
Assert::true($cli->sendto('localhost', $pm->getFreePort(), "packet-2"));
2020
Assert::false($cli->sendto('error_domain', $pm->getFreePort(), "hello"));
2121
Assert::assert($cli->errCode, SWOOLE_ERROR_DNSLOOKUP_RESOLVE_FAILED);
22-
$cli->sendto('localhost', $pm->getFreePort(), "packet-3");
22+
Assert::true($cli->sendto('localhost', $pm->getFreePort(), "packet-3"));
2323
echo "DONE\n";
2424
};
2525
$pm->childFunc = function () use ($pm) {

0 commit comments

Comments
 (0)