Skip to content

Commit

Permalink
Refactor uv callback signatures
Browse files Browse the repository at this point in the history
  • Loading branch information
bwoebi committed Feb 15, 2020
1 parent b05e148 commit e76ac0d
Show file tree
Hide file tree
Showing 13 changed files with 255 additions and 327 deletions.
498 changes: 223 additions & 275 deletions php_uv.c

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion tests/005-uv_listen_cb-not-destroyed.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class TcpServer
$client = uv_tcp_init($this->loop);
uv_accept($server, $client);

uv_read_start($client, function ($socket, $nread, $buffer) {
uv_read_start($client, function ($socket, $buffer) {
echo 'OK', PHP_EOL;
uv_close($socket);
});
Expand Down
12 changes: 6 additions & 6 deletions tests/300-fs.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ Check for fs read and close
define("FIXTURE_PATH", dirname(__FILE__) . "/fixtures/hello.data");

uv_fs_open(uv_default_loop(), FIXTURE_PATH, UV::O_RDONLY, 0, function($r) {
uv_fs_read(uv_default_loop(), $r, $offset = 0, $len = 32, function($stream, $nread, $data) {
if ($nread <= 0) {
if ($nread < 0) {
uv_fs_read(uv_default_loop(), $r, $offset = 0, $len = 32, function($stream, $data) {
if (is_long($data)) {
if ($data < 0) {
throw new Exception("read error");
}

Expand All @@ -20,9 +20,9 @@ uv_fs_open(uv_default_loop(), FIXTURE_PATH, UV::O_RDONLY, 0, function($r) {

// test offset
uv_fs_open(uv_default_loop(), FIXTURE_PATH, UV::O_RDONLY, 0, function($r) {
uv_fs_read(uv_default_loop(), $r, $offset = 1, $len = 32, function($stream, $nread, $data) {
if ($nread <= 0) {
if ($nread < 0) {
uv_fs_read(uv_default_loop(), $r, $offset = 1, $len = 32, function($stream, $data) {
if (is_long($data)) {
if ($data < 0) {
throw new Exception("read error");
}

Expand Down
2 changes: 1 addition & 1 deletion tests/310-fs-mkdir.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ uv_fs_mkdir(uv_default_loop(), DIRECTORY_PATH, 0755, function($result) {
uv_run();

--EXPECTF--
bool(true)
int(0)
2 changes: 1 addition & 1 deletion tests/311-fs-rmdir.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ uv_fs_rmdir(uv_default_loop(), DIRECTORY_PATH, function($result) {
uv_run();

--EXPECTF--
bool(true)
int(0)
12 changes: 3 additions & 9 deletions tests/320-fs-readlink.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,12 @@ uv_fs_readlink() segfaults if file not a link

$uv = uv_loop_new();

uv_fs_readlink($uv, __FILE__, function () {
var_dump(func_get_args());
uv_fs_readlink($uv, __FILE__, function ($result) {
var_dump($result < 0);
});

uv_run($uv);

?>
--EXPECT--
array(2) {
[0]=>
bool(false)
[1]=>
NULL
}

bool(true)
2 changes: 1 addition & 1 deletion tests/330-poll.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ HELO
EOF;
uv_write($client, $request, function($client, $stat) {
if ($stat == 0) {
uv_read_start($client, function($client, $nread, $buffer) {
uv_read_start($client, function($client, $buffer) {
echo "$buffer\n";
uv_close($client);
});
Expand Down
28 changes: 7 additions & 21 deletions tests/399-fs-stat-regression-no14.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,20 @@ Check for #14
<?php
$loop = uv_default_loop();
$filename ="does_not_exist.txt";
uv_fs_stat($loop, $filename, function ($result, $stat) use ($loop) {
if (!$result) {
echo 'OK' . PHP_EOL;
uv_fs_stat($loop, $filename, function ($stat) use ($loop) {
if (is_long($stat) && $stat < 0) {
echo 'OK' . PHP_EOL;
} else {
echo 'FAILED: uv_fs_stat should have returned false' . PHP_EOL;
}

if (is_null($stat)) {
echo "NULL" . PHP_EOL;
} else {
echo "FAILED: uv_fs_stat \$stat return value should be NULL" . PHP_EOL;
echo "FAILED: uv_fs_stat should have returned an array with values" . PHP_EOL;
}

$filename = tempnam(sys_get_temp_dir(), 'test-no14');

uv_fs_stat($loop, $filename, function ($result, $stat) {
if ($result) {
echo 'OK' . PHP_EOL;
} else {
echo "FAILED: uv_fs_stat should have returned true" . PHP_EOL;
}

if(!empty($stat)) {
uv_fs_stat($loop, $filename, function ($stat) {
if (is_array($stat) || !$stat) {
echo 'OK' . PHP_EOL;
} else {
echo 'FAILED: $stat should be an array with values' . PHP_EOL;
echo "FAILED: uv_fs_stat should have returned an array with values" . PHP_EOL;
}
});

Expand All @@ -39,6 +27,4 @@ uv_run();

--EXPECT--
OK
NULL
OK
OK
2 changes: 1 addition & 1 deletion tests/400-tcp_bind.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ uv_tcp_bind($tcp, uv_ip4_addr('0.0.0.0',0));
uv_listen($tcp, 100, function($server){
$client = uv_tcp_init();
uv_accept($server, $client);
uv_read_start($client, function($socket, $nread, $buffer) use ($server) {
uv_read_start($client, function($socket, $buffer) use ($server) {
echo $buffer . PHP_EOL;
uv_close($socket);
uv_close($server);
Expand Down
4 changes: 2 additions & 2 deletions tests/400-tcp_bind6.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ uv_tcp_bind6($tcp, uv_ip6_addr('::1', 0));
uv_listen($tcp,100, function($server) {
$client = uv_tcp_init();
uv_accept($server, $client);
uv_read_start($client, function($socket, $nread, $buffer) use ($server) {
uv_read_start($client, function($socket, $buffer) use ($server) {
echo $buffer . PHP_EOL;
uv_close($socket);
uv_close($server);
Expand All @@ -17,7 +17,7 @@ uv_listen($tcp,100, function($server) {
$addrinfo = uv_tcp_getsockname($tcp);

$c = uv_tcp_init();
uv_tcp_connect6($c, uv_ip6_addr($addrinfo['address'], $addrinfo['port']), function($client, $stat) {
uv_tcp_connect($c, uv_ip6_addr($addrinfo['address'], $addrinfo['port']), function($client, $stat) {
if ($stat == 0) {
uv_write($client,"Hello",function($socket, $stat){
uv_close($socket, function() { });
Expand Down
2 changes: 1 addition & 1 deletion tests/500-udp_bind.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Check for udp bind
$udp = uv_udp_init();
uv_udp_bind($udp, uv_ip4_addr('0.0.0.0', 10000));

uv_udp_recv_start($udp, function($stream, $nread, $buffer) {
uv_udp_recv_start($udp, function($stream, $buffer) {
echo "recv: " . $buffer;

uv_close($stream);
Expand Down
4 changes: 2 additions & 2 deletions tests/500-udp_bind6.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ Check for udp bind
$udp = uv_udp_init();
uv_udp_bind6($udp, uv_ip6_addr('::1',10000));

uv_udp_recv_start($udp,function($stream, $nread, $buffer) {
uv_udp_recv_start($udp,function($stream, $buffer) {
echo "recv: " . $buffer;

uv_close($stream);
});

$uv = uv_udp_init();
uv_udp_send6($uv, "Hello", uv_ip6_addr("::1", 10000), function($uv, $s) {
uv_udp_send($uv, "Hello", uv_ip6_addr("::1", 10000), function($uv, $s) {
uv_close($uv);
});

Expand Down
12 changes: 6 additions & 6 deletions tests/600-pipe_bind.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,22 @@ $ret = uv_pipe_bind($a, PIPE_PATH);
uv_listen($a, 8192, function($stream) {
$pipe = uv_pipe_init(uv_default_loop(), 0);
uv_accept($stream, $pipe);
uv_read_start($pipe,function($pipe, $nread, $buffer) use ($stream) {
if ($nread === \UV::EOF) {
uv_read_start($pipe,function($pipe, $data) use ($stream) {
if ($data === \UV::EOF) {
return;
}

echo $buffer;
echo $data;
uv_read_stop($pipe);
uv_close($stream, function() {
@unlink(PIPE_PATH);
});
});
});

$b = uv_pipe_init(uv_default_loop(), 0);
uv_pipe_connect($b, PIPE_PATH, function($a, $b) {
uv_write($b, "Hello", function($stream, $stat) {
$pipe = uv_pipe_init(uv_default_loop(), 0);
uv_pipe_connect($pipe, PIPE_PATH, function($pipe, $status) {
uv_write($pipe, "Hello", function($stream, $stat) {
uv_close($stream);
});
});
Expand Down

1 comment on commit e76ac0d

@bwoebi
Copy link
Member Author

@bwoebi bwoebi commented on e76ac0d Feb 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary motivation was to expose the return status within fs functions, which were usually bool. So, if I changed that, you anyway needed to distinguish the type (because with long, 0 is the success state and anything else failure). At that point I would anyway need to introduce a new version of the library.
At the same time I paid attention that you can actually just work around it with a simple type check.

Yeah, the coalescing of uv_read_start was not strictly necessary but then again, I wanted to bring it in line with fs functions.

And also - I thought we are now in progress of actually creating stops and revamping the docs. Thus I thought it would be a good idea to clean up now instead of only afterwards or even never.

Please sign in to comment.