Skip to content

Commit dd8b8fe

Browse files
bigbesbigbes
committed
First bunch of review fixes
1 parent 6453c4b commit dd8b8fe

11 files changed

+75
-99
lines changed

‎.travis.yml

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ language: php
77
php:
88
- 7.0
99
- 7.1
10+
- 7.2
11+
- 7.3
1012
- nightly
1113

1214
python:

‎README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ Tarantool class instance
156156
$tnt = new Tarantool(); // -> new Tarantool('localhost', 3301);
157157
$tnt = new Tarantool('tcp://test:test@localhost');
158158
$tnt = new Tarantool('tcp://test:test@localhost:3301');
159-
$tnt = new Tarantool('test:test@unix/:/var/tmp/tarantool.sock);
160-
$tnt = new Tarantool('unix:///var/tmp/tarantool.sock); /* if no login is needed */
159+
$tnt = new Tarantool('test:test@unix/:/var/tmp/tarantool.sock');
160+
$tnt = new Tarantool('unix:///var/tmp/tarantool.sock'); /* if no login is needed */
161161
```
162162

163163
## Manipulation connection

‎lib/tarantool_server.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ def find_exe(self):
211211
raise RuntimeError("Can't find server executable in " + os.environ["PATH"])
212212

213213
def generate_configuration(self):
214-
# print(self.args)
215214
os.putenv("PRIMARY_PORT", str(self.args['primary']))
216215
os.putenv("ADMIN_PORT", str(self.args['admin']))
217216

@@ -261,7 +260,7 @@ def start(self):
261260
self.generate_configuration()
262261
if self.script:
263262
shutil.copy(self.script, self.script_dst)
264-
os.chmod(self.script_dst, 511)
263+
os.chmod(self.script_dst, 0o777)
265264
args = self.prepare_args()
266265
self.process = subprocess.Popen(args,
267266
cwd = self.vardir,
@@ -284,4 +283,4 @@ def clean(self):
284283

285284
def __del__(self):
286285
self.stop()
287-
# self.clean()
286+
self.clean()

‎src/php_tarantool.h

+4
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,11 @@ ZEND_EXTERN_MODULE_GLOBALS(tarantool);
9595

9696
typedef struct tarantool_object {
9797
struct tarantool_connection {
98+
/* physical url, that's used for connection in raw PHP obtained
99+
* from parsed url by `tarantool_url_write_php_format` */
98100
char *url;
101+
/* parsed url, after have user/password/...
102+
* obtained from user string by `tarantool_url_parse` */
99103
struct tarantool_url *url_parsed;
100104
php_stream *stream;
101105
struct tarantool_schema *schema;

‎src/tarantool.c

+19-45
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,6 @@ pid_gen(struct tarantool_url *url, const char *user, const char *prefix,
154154
const char *suffix, size_t suffix_len, size_t *olen) {
155155

156156
char *plist_id = NULL, *tmp = NULL;
157-
/* if user is not defined, then user is 'guest' */
158157
int len = 0;
159158
if (url->type == TARANTOOL_URL_TCP) {
160159
len = spprintf(&plist_id, 0, "tarantool-%s:id=%s:%d-%s",
@@ -265,8 +264,8 @@ static int __tarantool_connect(tarantool_object *t_obj) {
265264
continue;
266265
if (tntll_stream_read2(obj->stream, obj->greeting,
267266
GREETING_SIZE) != GREETING_SIZE) {
268-
// int errno = php_stream_errno();
269-
spprintf(&err, 0, "Fails to read greeting [%d]: %s", errno, strerror(errno));
267+
spprintf(&err, 0, "Fails to read greeting [%d]: %s",
268+
errno, strerror(errno));
270269
continue;
271270
}
272271
if (php_tp_verify_greetings(obj->greeting) == 0) {
@@ -278,7 +277,7 @@ static int __tarantool_connect(tarantool_object *t_obj) {
278277
}
279278
if (count == 0) {
280279
ioexception:
281-
tarantool_throw_ioexception("%s ", err);
280+
tarantool_throw_ioexception("%s", err);
282281
efree(err);
283282
return FAILURE;
284283
}
@@ -294,7 +293,7 @@ inline static int __tarantool_reconnect(tarantool_object *t_obj) {
294293
return __tarantool_connect(t_obj);
295294
}
296295

297-
static void
296+
static void
298297
tarantool_connection_free(tarantool_connection *obj, int is_persistent
299298
TSRMLS_DC) {
300299
if (obj == NULL)
@@ -303,7 +302,7 @@ tarantool_connection_free(tarantool_connection *obj, int is_persistent
303302
pefree(obj->greeting, is_persistent);
304303
obj->greeting = NULL;
305304
}
306-
// tarantool_stream_close(obj);
305+
tarantool_stream_close(obj);
307306
if (obj->persistent_id) {
308307
zend_string_release(obj->persistent_id);
309308
obj->persistent_id = NULL;
@@ -626,10 +625,11 @@ int obtain_space_by_spaceno(tarantool_connection *obj, uint32_t space_no) {
626625
size_t body_size = php_mp_unpack_package_size(pack_len);
627626
smart_string_ensure(obj->value, body_size);
628627
if (tarantool_stream_read(obj, obj->value->c,
629-
body_size) == FAILURE)
628+
body_size) == FAILURE)
630629
return FAILURE;
631630

632-
struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
631+
struct tnt_response resp;
632+
memset(&resp, 0, sizeof(struct tnt_response));
633633
if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
634634
tarantool_throw_parsingexception("query");
635635
return FAILURE;
@@ -682,10 +682,11 @@ int get_spaceno_by_name(tarantool_connection *obj, zval *name) {
682682
size_t body_size = php_mp_unpack_package_size(pack_len);
683683
smart_string_ensure(obj->value, body_size);
684684
if (tarantool_stream_read(obj, obj->value->c,
685-
body_size) == FAILURE)
685+
body_size) == FAILURE)
686686
return FAILURE;
687687

688-
struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
688+
struct tnt_response resp;
689+
memset(&resp, 0, sizeof(struct tnt_response));
689690
if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
690691
tarantool_throw_parsingexception("query");
691692
return FAILURE;
@@ -749,7 +750,8 @@ int get_indexno_by_name(tarantool_connection *obj, int space_no,
749750
if (tarantool_stream_read(obj, obj->value->c, body_size) == FAILURE)
750751
return FAILURE;
751752

752-
struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
753+
struct tnt_response resp;
754+
memset(&resp, 0, sizeof(struct tnt_response));
753755
if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
754756
tarantool_throw_parsingexception("query");
755757
return FAILURE;
@@ -804,7 +806,8 @@ int get_fieldno_by_name(tarantool_connection *obj, uint32_t space_no,
804806
if (tarantool_stream_read(obj, obj->value->c, body_size) == FAILURE)
805807
return FAILURE;
806808

807-
struct tnt_response resp; memset(&resp, 0, sizeof(struct tnt_response));
809+
struct tnt_response resp;
810+
memset(&resp, 0, sizeof(struct tnt_response));
808811
if (php_tp_response(&resp, obj->value->c, body_size) == -1) {
809812
tarantool_throw_parsingexception("query");
810813
return FAILURE;
@@ -1119,9 +1122,9 @@ PHP_METHOD(Tarantool, __construct) {
11191122
suffix_or_port = &suffix_or_port_default;
11201123
}
11211124

1122-
if (strstr(host, "tcp://" ) != 0 ||
1123-
strstr(host, "unix://") != 0 ||
1124-
strstr(host, "unix/:" ) != 0 ||
1125+
if (strstr(host, "tcp://" ) != NULL ||
1126+
strstr(host, "unix://") != NULL ||
1127+
strstr(host, "unix/:" ) != NULL ||
11251128
Z_TYPE_P(suffix_or_port) == IS_STRING) {
11261129
url = estrdup(host);
11271130
url_len = host_len;
@@ -1173,34 +1176,6 @@ PHP_METHOD(Tarantool, __construct) {
11731176
if (url_parsed == NULL) {
11741177
THROW_EXC("failed to parse url: '%s'", url);
11751178
RETURN_FALSE;
1176-
} else {
1177-
/* set default user
1178-
if (url_parsed->user == NULL) {
1179-
url_parsed->user = estrdup("guest");
1180-
} */
1181-
/* set default password
1182-
if (url_parsed->pass && strlen(url_parsed->pass) == 0) {
1183-
efree(url_parsed->pass);
1184-
url_parsed->pass = NULL;
1185-
} */
1186-
/* try to deduct scheme (based on host/path presence)
1187-
if (url_parsed->scheme == NULL) {
1188-
if (url_parsed->host != NULL) {
1189-
url_parsed->scheme = estrdup("tcp");
1190-
} else if (url_parsed->path != NULL) {
1191-
url_parsed->scheme = estrdup("unix");
1192-
} else {
1193-
THROW_EXC("Unknown url: %s", url);
1194-
RETURN_FALSE;
1195-
}
1196-
} */
1197-
/* check that protocik is supported
1198-
if (strcmp(url_parsed->scheme, "tcp") != 0 &&
1199-
strcmp(url_parsed->scheme, "unix") != 0) {
1200-
THROW_EXC("Unsupported protocol: %s",
1201-
url_parsed->scheme);
1202-
RETURN_FALSE;
1203-
} */
12041179
}
12051180
efree(url);
12061181

@@ -1212,7 +1187,7 @@ PHP_METHOD(Tarantool, __construct) {
12121187

12131188
le = zend_hash_find_ptr(&EG(persistent_list), plist_id);
12141189
if (le != NULL) {
1215-
/* It's likely §*/
1190+
/* It's likely */
12161191
if (le->type == php_tarantool_list_entry()) {
12171192
obj = (tarantool_connection *) le->ptr;
12181193
plist_new_entry = false;
@@ -1245,7 +1220,6 @@ PHP_METHOD(Tarantool, __construct) {
12451220
char *url_s = pestrdup(obj->url, is_persistent);
12461221
efree(obj->url);
12471222
obj->url = url_s;
1248-
/* If pass == NULL, then authenticate without password */
12491223
if (is_persistent) {
12501224
obj->persistent_id = pid_pzsgen(obj->url_parsed,
12511225
obj->url_parsed->user,

‎src/tarantool_schema.c

+19-14
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,24 @@
1414
#define MUR_SEED 13
1515
#include "third_party/msgpuck.h"
1616

17-
#define MH_DEFINE_CMPFUNC(NAME, TYPE) \
18-
int mh_##NAME##cmp_eq(const TYPE **lval, const TYPE **rval, void *arg) { \
19-
(void *) arg; \
20-
if ((*lval)->key.id_len != (*rval)->key.id_len) \
21-
return 0; \
22-
return !memcmp((*lval)->key.id, (*rval)->key.id, (*rval)->key.id_len); \
23-
} \
24-
\
25-
int mh_##NAME##cmp_key_eq(const struct schema_key *key, const TYPE **val, void *arg) { \
26-
(void *) arg; \
27-
if (key->id_len != (*val)->key.id_len) \
28-
return 0; \
29-
return !memcmp(key->id, (*val)->key.id, key->id_len); \
17+
#define MH_DEFINE_CMPFUNC(NAME, TYPE) \
18+
int mh_##NAME##cmp_eq(const TYPE **lval, const TYPE **rval, void *arg)\
19+
{ \
20+
(void *) arg; \
21+
if ((*lval)->key.id_len != (*rval)->key.id_len) \
22+
return 0; \
23+
return !memcmp((*lval)->key.id, (*rval)->key.id, \
24+
(*rval)->key.id_len); \
25+
} \
26+
\
27+
int mh_##NAME##cmp_key_eq(const struct schema_key *key, \
28+
const TYPE **val, void *arg) { \
29+
(void *) arg; \
30+
if (key->id_len != (*val)->key.id_len) \
31+
return 0; \
32+
return !memcmp(key->id, (*val)->key.id, key->id_len); \
3033
}
34+
3135
MH_DEFINE_CMPFUNC(field, struct schema_field_value);
3236
MH_DEFINE_CMPFUNC(index, struct schema_index_value);
3337
MH_DEFINE_CMPFUNC(space, struct schema_space_value);
@@ -37,7 +41,8 @@ MH_DEFINE_CMPFUNC(space, struct schema_space_value);
3741

3842
#define mh_eq(a, b, arg) mh_fieldcmp_eq(a, b, arg)
3943
#define mh_eq_key(a, b, arg) mh_fieldcmp_key_eq(a, b, arg)
40-
#define mh_hash(x, arg) PMurHash32(MUR_SEED, (*x)->key.id, (*x)->key.id_len)
44+
#define mh_hash(x, arg) PMurHash32(MUR_SEED, (*x)->key.id, \
45+
(*x)->key.id_len)
4146
#define mh_hash_key(x, arg) PMurHash32(MUR_SEED, (x)->id, (x)->id_len);
4247

4348
#define mh_node_t struct schema_field_value *

‎src/tarantool_url.c

+4-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ url_part_copy(const char *part, size_t part_len, int persistent) {
1616

1717
struct tarantool_url *
1818
tarantool_url_parse(const char *url, int persistent) {
19-
struct uri parsed; memset(&parsed, 0, sizeof(struct uri));
19+
struct uri parsed;
20+
memset(&parsed, 0, sizeof(struct uri));
2021

2122
if (uri_parse(&parsed, url) == -1)
2223
return NULL;
@@ -67,8 +68,8 @@ tarantool_url_parse(const char *url, int persistent) {
6768
host_len = parsed.host_len;
6869
}
6970
if (parsed.service == NULL) {
70-
port = "3303";
71-
port_len = sizeof("3303") - 1;
71+
port = "3301";
72+
port_len = sizeof("3301") - 1;
7273
} else {
7374
port = parsed.service;
7475
port_len = parsed.service_len;

‎src/tarantool_url.h

+3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ tarantool_url_parse(const char *url, int persistent);
2525
void
2626
tarantool_url_free(struct tarantool_url *url, int persistent);
2727

28+
/*
29+
* Construct url in PHP format (without user/password)
30+
*/
2831
const char *
2932
tarantool_url_write_php_format(struct tarantool_url *url, int persistent);
3033

‎test-run.py

+15-20
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
11
#!/usr/bin/env python
2+
# pylint: disable=missing-docstring
3+
from __future__ import print_function
24

35
import os
46
import sys
5-
import shlex
67
import shutil
78
import subprocess
89

9-
import time
10-
1110
from lib.tarantool_server import TarantoolServer
1211

13-
from pprint import pprint
14-
1512
PHPUNIT_PHAR = 'phpunit.phar'
1613

1714
def read_popen(cmd):
@@ -25,9 +22,9 @@ def read_popen_config(cmd):
2522

2623
def find_php_bin():
2724
path = read_popen('which php').strip().decode()
28-
if (path.find('phpenv') != -1):
25+
if path.find('phpenv') != -1:
2926
version = read_popen('phpenv global')
30-
if (version.find('system') != -1):
27+
if version.find('system') != -1:
3128
return '/usr/bin/php'
3229
return '~/.phpenv/versions/{0}/bin/php'.format(version.strip())
3330
return path
@@ -38,9 +35,9 @@ def prepare_env(php_ini):
3835
if not os.path.isdir('var') and not os.path.exists('var'):
3936
os.mkdir('var')
4037
shutil.copy('test/shared/phpunit.xml', 'var')
41-
test_dir_path = os.path.abspath(os.path.join(os.getcwd(), 'test'))
42-
test_lib_path = os.path.join(test_dir_path, PHPUNIT_PHAR)
43-
# shutil.copy('test/shared/tarantool.ini', 'var')
38+
# test_dir_path = os.path.abspath(os.path.join(os.getcwd(), 'test'))
39+
# test_lib_path = os.path.join(test_dir_path, PHPUNIT_PHAR)
40+
# shutil.copy('test/shared/tarantool.ini', 'var')
4441
shutil.copy(php_ini, 'var')
4542
shutil.copy('modules/tarantool.so', 'var')
4643

@@ -60,9 +57,9 @@ def run_tests(vardir):
6057
print('Running against ' + version)
6158

6259
for php_ini in [
63-
'test/shared/tarantool-1.ini'
64-
# 'test/shared/tarantool-2.ini',
65-
# 'test/shared/tarantool-3.ini'
60+
'test/shared/tarantool-1.ini',
61+
'test/shared/tarantool-2.ini',
62+
'test/shared/tarantool-3.ini'
6663
]:
6764
cmd = ''
6865
shutil.copy(php_ini, os.path.join(test_cwd, 'tarantool.ini'))
@@ -98,20 +95,18 @@ def run_tests(vardir):
9895

9996
print('Running "%s" with "%s"' % (cmd, php_ini))
10097
proc = subprocess.Popen(cmd, shell=True, cwd=test_cwd)
101-
rv = proc.wait()
102-
if rv != 0:
98+
if proc.wait() != 0:
10399
print('Error')
104-
return -1
100+
return
105101
if '--gdb' in sys.argv or '--lldb' in sys.argv:
106-
return -1
102+
return
107103

108104
finally:
109-
a = [
105+
for elem in [
110106
os.path.join(test_cwd, 'tarantool.ini'),
111107
os.path.join(test_cwd, 'phpunit.xml'),
112108
os.path.join(test_cwd, 'tarantool.so'),
113-
]
114-
for elem in a:
109+
]:
115110
if os.path.exists(elem):
116111
os.remove(elem)
117112

0 commit comments

Comments
 (0)