From 5c760962496c17a98ca5a976cfcfe576bbdd380c Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 22 Nov 2024 11:55:37 +0000 Subject: [PATCH 1/4] Fix indentation and spaces Use space instead of tabs for coherence with other part of the code. Avoid spaces at the end of the line. Signed-off-by: Frediano Ziglio --- .../lib/xapi-stdext-unix/unixext_open_stubs.c | 2 +- .../lib/xapi-stdext-unix/unixext_stubs.c | 98 +++++++++---------- .../xapi-stdext-unix/unixext_write_stubs.c | 8 +- 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c index d15cfeff0b1..62ff2f71055 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c @@ -51,7 +51,7 @@ CAMLprim value stub_stdext_unix_open_direct(value path, value flags, value perm) char * p; cv_flags = caml_convert_flag_list(flags, open_flag_table); - + #ifdef O_DIRECT cv_flags |= O_DIRECT; #endif diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_stubs.c b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_stubs.c index 28fd7f9af89..9b0299fac3e 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_stubs.c +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_stubs.c @@ -23,7 +23,7 @@ #include #include #if defined(__linux__) -# include +# include #endif #include @@ -41,28 +41,28 @@ /* Set the TCP_NODELAY flag on a Unix.file_descr */ CAMLprim value stub_unixext_set_tcp_nodelay (value fd, value bool) { - CAMLparam2 (fd, bool); - int c_fd = Int_val(fd); - int opt = (Bool_val(bool)) ? 1 : 0; - if (setsockopt(c_fd, IPPROTO_TCP, TCP_NODELAY, (void *)&opt, sizeof(opt)) != 0){ - uerror("setsockopt", Nothing); - } - CAMLreturn(Val_unit); + CAMLparam2 (fd, bool); + int c_fd = Int_val(fd); + int opt = (Bool_val(bool)) ? 1 : 0; + if (setsockopt(c_fd, IPPROTO_TCP, TCP_NODELAY, (void *)&opt, sizeof(opt)) != 0){ + uerror("setsockopt", Nothing); + } + CAMLreturn(Val_unit); } CAMLprim value stub_unixext_fsync (value fd) { - CAMLparam1(fd); - int c_fd = Int_val(fd); - int rc; - - caml_release_runtime_system(); - rc = fsync(c_fd); - caml_acquire_runtime_system(); - if (rc != 0) uerror("fsync", Nothing); - CAMLreturn(Val_unit); + CAMLparam1(fd); + int c_fd = Int_val(fd); + int rc; + + caml_release_runtime_system(); + rc = fsync(c_fd); + caml_acquire_runtime_system(); + if (rc != 0) uerror("fsync", Nothing); + CAMLreturn(Val_unit); } - + CAMLprim value stub_unixext_blkgetsize64(value fd) { @@ -84,10 +84,10 @@ CAMLprim value stub_unixext_blkgetsize64(value fd) CAMLprim value stub_unixext_get_max_fd (value unit) { - CAMLparam1 (unit); - long maxfd; - maxfd = sysconf(_SC_OPEN_MAX); - CAMLreturn(Val_int(maxfd)); + CAMLparam1 (unit); + long maxfd; + maxfd = sysconf(_SC_OPEN_MAX); + CAMLreturn(Val_int(maxfd)); } #if defined(__linux__) @@ -102,41 +102,41 @@ CAMLprim value stub_unixext_set_sock_keepalives(value fd, value count, value idl { CAMLparam4(fd, count, idle, interval); - int c_fd = Int_val(fd); - int optval; - socklen_t optlen=sizeof(optval); - - optval = Int_val(count); - if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPCNT, &optval, optlen) < 0) { - uerror("setsockopt(TCP_KEEPCNT)", Nothing); - } -#if defined(__linux__) - optval = Int_val(idle); - if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPIDLE, &optval, optlen) < 0) { - uerror("setsockopt(TCP_KEEPIDLE)", Nothing); - } + int c_fd = Int_val(fd); + int optval; + socklen_t optlen=sizeof(optval); + + optval = Int_val(count); + if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPCNT, &optval, optlen) < 0) { + uerror("setsockopt(TCP_KEEPCNT)", Nothing); + } +#if defined(__linux__) + optval = Int_val(idle); + if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPIDLE, &optval, optlen) < 0) { + uerror("setsockopt(TCP_KEEPIDLE)", Nothing); + } #endif - optval = Int_val(interval); - if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPINTVL, &optval, optlen) < 0) { - uerror("setsockopt(TCP_KEEPINTVL)", Nothing); - } + optval = Int_val(interval); + if(setsockopt(c_fd, TCP_LEVEL, TCP_KEEPINTVL, &optval, optlen) < 0) { + uerror("setsockopt(TCP_KEEPINTVL)", Nothing); + } - CAMLreturn(Val_unit); + CAMLreturn(Val_unit); } void unixext_error(int code) { - static const value *exn = NULL; - - if (!exn) { - exn = caml_named_value("unixext.unix_error"); - if (!exn) - caml_invalid_argument("unixext.unix_error not initialiazed"); - } - caml_raise_with_arg(*exn, Val_int(code)); + static const value *exn = NULL; + + if (!exn) { + exn = caml_named_value("unixext.unix_error"); + if (!exn) + caml_invalid_argument("unixext.unix_error not initialiazed"); + } + caml_raise_with_arg(*exn, Val_int(code)); } -CAMLprim value stub_statvfs(value filename) +CAMLprim value stub_statvfs(value filename) { CAMLparam1(filename); CAMLlocal1(v); diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_write_stubs.c b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_write_stubs.c index e4be9f68018..b74a13e909e 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_write_stubs.c +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_write_stubs.c @@ -41,15 +41,15 @@ CAMLprim value stub_stdext_unix_write(value fd, value buf, value vofs, value vle written = 0; while (len > 0) { numbytes = len > UNIX_BUFFER_SIZE ? UNIX_BUFFER_SIZE : len; - ret = posix_memalign(&iobuf, PAGE_SIZE, numbytes); - if (ret != 0) - uerror("write/posix_memalign", Nothing); + ret = posix_memalign(&iobuf, PAGE_SIZE, numbytes); + if (ret != 0) + uerror("write/posix_memalign", Nothing); memmove (iobuf, &Byte(buf, ofs), numbytes); caml_enter_blocking_section(); ret = write(Int_val(fd), iobuf, numbytes); caml_leave_blocking_section(); - free(iobuf); + free(iobuf); if (ret == -1) { if ((errno == EAGAIN || errno == EWOULDBLOCK) && written > 0) break; From c254c5c4a215205b5ccf48de273b5da127fd8f6a Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 22 Nov 2024 11:56:58 +0000 Subject: [PATCH 2/4] Use caml_stat_strdup instead of caml_stat_alloc+strcpy Simplify code, avoid computing string length multiple times. Signed-off-by: Frediano Ziglio --- .../libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c index 62ff2f71055..b8ae75bcd36 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c @@ -55,8 +55,7 @@ CAMLprim value stub_stdext_unix_open_direct(value path, value flags, value perm) #ifdef O_DIRECT cv_flags |= O_DIRECT; #endif - p = caml_stat_alloc(caml_string_length(path) + 1); - strcpy(p, String_val(path)); + p = caml_stat_strdup(String_val(path)); /* open on a named FIFO can block (PR#1533) */ caml_enter_blocking_section(); fd = open(p, cv_flags, Int_val(perm)); From 5e3f5ccd90940733d058fce90145267e4fda05e9 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 22 Nov 2024 11:57:46 +0000 Subject: [PATCH 3/4] Call caml_stat_free inside blocking section Minor optimization, do the operation without engine lock. Signed-off-by: Frediano Ziglio --- .../libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c index b8ae75bcd36..9bf7cf79b5b 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext_open_stubs.c @@ -63,8 +63,8 @@ CAMLprim value stub_stdext_unix_open_direct(value path, value flags, value perm) if (fd != -1) ret = fcntl(fd, F_NOCACHE); #endif - caml_leave_blocking_section(); caml_stat_free(p); + caml_leave_blocking_section(); if (fd == -1) uerror("open", path); #ifndef O_DIRECT if (ret == -1) uerror("fcntl", path); From d7ebcdba84dfa889fd9278bda5fc08f2349df525 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 22 Nov 2024 14:26:30 +0000 Subject: [PATCH 4/4] Add a test for Unixext.Direct module Very simple test. Exercise modified code. Signed-off-by: Frediano Ziglio --- .../lib/xapi-stdext-unix/test/unixext_test.ml | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/unixext_test.ml b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/unixext_test.ml index be848f7b8a4..83983999541 100644 --- a/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/unixext_test.ml +++ b/ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/test/unixext_test.ml @@ -281,8 +281,26 @@ let test_select = in true +let test_direct = + let gen = Gen.unit in + Test.make ~count:1 ~name:__FUNCTION__ gen @@ fun _ -> + let fn = "test_file_direct" in + let buf = Bytes.make 10000 'x' in + Unixext.Direct.with_openfile fn [Unix.O_RDWR; Unix.O_CREAT] 0o600 (fun f -> + let res = Unixext.Direct.write f buf 0 4096 in + QCheck2.assume (res = 4096) + ) ; + Unix.unlink fn ; + true + let tests = - [test_select; test_proxy; test_time_limited_write; test_time_limited_read] + [ + test_select + ; test_proxy + ; test_time_limited_write + ; test_time_limited_read + ; test_direct + ] let () = (* avoid SIGPIPE *)