Skip to content

Commit

Permalink
No-op instead of error on empty read/write (nodejs#261)
Browse files Browse the repository at this point in the history
* No-op instead of error on empty read/write

This commit inserts a special case to `fd_{read,write,pread,pwrite}`
such that passing in an empty list of buffers results in a no-op.  This
behavior is consistent with Linux host as well as other Wasm runtimes.

fixes nodejs#260
  • Loading branch information
yagehu authored Apr 22, 2024
1 parent d420a05 commit 81ac54a
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 5 deletions.
41 changes: 36 additions & 5 deletions src/uvwasi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ uvwasi_errno_t uvwasi_fd_pread(uvwasi_t* uvwasi,
offset,
nread);

if (uvwasi == NULL || iovs == NULL || nread == NULL || offset > INT64_MAX)
if (uvwasi == NULL || (iovs == NULL && iovs_len > 0) || nread == NULL || offset > INT64_MAX)
return UVWASI_EINVAL;

err = uvwasi_fd_table_get(uvwasi->fds,
Expand All @@ -1169,6 +1169,14 @@ uvwasi_errno_t uvwasi_fd_pread(uvwasi_t* uvwasi,
if (err != UVWASI_ESUCCESS)
return err;

// libuv returns EINVAL in this case. To behave consistently with other
// Wasm runtimes, return OK here with a no-op.
if (iovs_len == 0) {
uv_mutex_unlock(&wrap->mutex);
*nread = 0;
return UVWASI_ESUCCESS;
}

err = uvwasi__setup_iovs(uvwasi, &bufs, iovs, iovs_len);
if (err != UVWASI_ESUCCESS) {
uv_mutex_unlock(&wrap->mutex);
Expand Down Expand Up @@ -1282,7 +1290,7 @@ uvwasi_errno_t uvwasi_fd_pwrite(uvwasi_t* uvwasi,
offset,
nwritten);

if (uvwasi == NULL || iovs == NULL || nwritten == NULL || offset > INT64_MAX)
if (uvwasi == NULL || (iovs == NULL && iovs_len > 0) || nwritten == NULL || offset > INT64_MAX)
return UVWASI_EINVAL;

err = uvwasi_fd_table_get(uvwasi->fds,
Expand All @@ -1293,6 +1301,14 @@ uvwasi_errno_t uvwasi_fd_pwrite(uvwasi_t* uvwasi,
if (err != UVWASI_ESUCCESS)
return err;

// libuv returns EINVAL in this case. To behave consistently with other
// Wasm runtimes, return OK here with a no-op.
if (iovs_len == 0) {
uv_mutex_unlock(&wrap->mutex);
*nwritten = 0;
return UVWASI_ESUCCESS;
}

err = uvwasi__setup_ciovs(uvwasi, &bufs, iovs, iovs_len);
if (err != UVWASI_ESUCCESS) {
uv_mutex_unlock(&wrap->mutex);
Expand Down Expand Up @@ -1332,14 +1348,21 @@ uvwasi_errno_t uvwasi_fd_read(uvwasi_t* uvwasi,
iovs,
iovs_len,
nread);

if (uvwasi == NULL || iovs == NULL || nread == NULL)
if (uvwasi == NULL || (iovs == NULL && iovs_len > 0) || nread == NULL)
return UVWASI_EINVAL;

err = uvwasi_fd_table_get(uvwasi->fds, fd, &wrap, UVWASI_RIGHT_FD_READ, 0);
if (err != UVWASI_ESUCCESS)
return err;

// libuv returns EINVAL in this case. To behave consistently with other
// Wasm runtimes, return OK here with a no-op.
if (iovs_len == 0) {
uv_mutex_unlock(&wrap->mutex);
*nread = 0;
return UVWASI_ESUCCESS;
}

err = uvwasi__setup_iovs(uvwasi, &bufs, iovs, iovs_len);
if (err != UVWASI_ESUCCESS) {
uv_mutex_unlock(&wrap->mutex);
Expand Down Expand Up @@ -1634,13 +1657,21 @@ uvwasi_errno_t uvwasi_fd_write(uvwasi_t* uvwasi,
iovs_len,
nwritten);

if (uvwasi == NULL || iovs == NULL || nwritten == NULL)
if (uvwasi == NULL || (iovs == NULL && iovs_len > 0) || nwritten == NULL)
return UVWASI_EINVAL;

err = uvwasi_fd_table_get(uvwasi->fds, fd, &wrap, UVWASI_RIGHT_FD_WRITE, 0);
if (err != UVWASI_ESUCCESS)
return err;

// libuv returns EINVAL in this case. To behave consistently with other
// Wasm runtimes, return OK here with a no-op.
if (iovs_len == 0) {
uv_mutex_unlock(&wrap->mutex);
*nwritten = 0;
return UVWASI_ESUCCESS;
}

err = uvwasi__setup_ciovs(uvwasi, &bufs, iovs, iovs_len);
if (err != UVWASI_ESUCCESS) {
uv_mutex_unlock(&wrap->mutex);
Expand Down
82 changes: 82 additions & 0 deletions test/test-fd-read-empty.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
#include <assert.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include "uvwasi.h"
#include "uv.h"
#include "test-common.h"

#define TEST_TMP_DIR "./out/tmp"

int main(void) {
const char* path = "file.txt";
uvwasi_t uvwasi;
uvwasi_iovec_t* iovs;
uvwasi_fd_t fd;
uvwasi_options_t init_options;
uvwasi_rights_t fs_rights_base;
uvwasi_size_t nwritten;
uvwasi_size_t iovs_len = 0;
uvwasi_errno_t err;
uv_fs_t req;
int r;

setup_test_environment();

r = uv_fs_mkdir(NULL, &req, TEST_TMP_DIR, 0777, NULL);
uv_fs_req_cleanup(&req);
assert(r == 0 || r == UV_EEXIST);

uvwasi_options_init(&init_options);
init_options.preopenc = 1;
init_options.preopens = calloc(1, sizeof(uvwasi_preopen_t));
init_options.preopens[0].mapped_path = "/var";
init_options.preopens[0].real_path = TEST_TMP_DIR;

err = uvwasi_init(&uvwasi, &init_options);
assert(err == 0);

// Create a file.
fs_rights_base = UVWASI_RIGHT_FD_DATASYNC |
UVWASI_RIGHT_FD_FILESTAT_GET |
UVWASI_RIGHT_FD_FILESTAT_SET_SIZE |
UVWASI_RIGHT_FD_READ |
UVWASI_RIGHT_FD_SEEK |
UVWASI_RIGHT_FD_SYNC |
UVWASI_RIGHT_FD_TELL |
UVWASI_RIGHT_FD_WRITE |
UVWASI_RIGHT_PATH_UNLINK_FILE;
err = uvwasi_path_open(&uvwasi,
3,
1,
path,
strlen(path) + 1,
UVWASI_O_CREAT,
fs_rights_base,
0,
0,
&fd);
assert(err == 0);

iovs = calloc(iovs_len, sizeof(uvwasi_ciovec_t));
assert(iovs != NULL);

for (int i = 0; i < iovs_len; i++) {
iovs[i].buf_len = 0;
iovs[i].buf = malloc(0);
}

err = uvwasi_fd_read(&uvwasi, fd, iovs, iovs_len, &nwritten);
assert(err == UVWASI_ESUCCESS);

uvwasi_destroy(&uvwasi);

for (int i = 0; i < iovs_len; i++) {
free((void *) iovs[i].buf);
}

free(iovs);
free(init_options.preopens);

return 0;
}

0 comments on commit 81ac54a

Please sign in to comment.