The callback doesn't make much sense: Could the value change per-connection? Unlikely. This is a property of the plugin as a whole. I changed the name to "errno_is_preserved", because it's not about the reliability of errno, but about whether errno is preserved across calls. --- docs/nbdkit-plugin.pod | 37 +++++++++------------------------ include/nbdkit-plugin.h | 2 +- plugins/ocaml/nbdkit-ocaml-plugin.pod | 11 ---------- plugins/ocaml/ocaml.c | 11 ---------- plugins/perl/nbdkit-perl-plugin.pod | 6 ------ plugins/perl/perl.c | 12 ----------- plugins/python/nbdkit-python-plugin.pod | 6 ------ plugins/python/python.c | 12 ----------- plugins/ruby/nbdkit-ruby-plugin.pod | 6 ------ plugins/ruby/ruby.c | 12 ----------- src/connections.c | 7 ++----- src/internal.h | 3 +-- src/plugins.c | 26 +++++++++-------------- 13 files changed, 24 insertions(+), 127 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 3611244..4b364f3 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -147,10 +147,16 @@ C<nbdkit_error> to report an error message; additionally, if the callback is involved in serving data, the plugin should call C<nbdkit_set_error> to influence the error code that will be sent to the client. These two functions can be called in either order. Then, -the callback should return the appropriate error indication, eg. NULL -or -1. If the call to C<nbdkit_set_error> is omitted while serving -data, then the C<.errno_is_reliable> callback controls whether to fall -back to the value of C<errno> or a hard-coded C<EIO>. +the callback should return the appropriate error indication, +eg. C<NULL> or C<-1>. + +If the call to C<nbdkit_set_error> is omitted while serving data, then +the global variable C<errno> may be used. For plugins which have +C<.errno_is_preserved == 1> the core code will use C<errno>. In +plugins written in non-C languages, we usually cannot trust that +C<errno> will not be overwritten when returning from that language to +C. In that case, either the plugin must call C<nbdkit_set_error> or +hard-coded C<EIO> is used. C<nbdkit_error> has the following prototype and works like L<printf(3)>: @@ -386,29 +392,6 @@ error message and return C<-1>. This callback is not required. If omitted, then we return true iff a C<.trim> callback has been defined. -=head2 C<.errno_is_reliable> - - int errno_is_reliable (void *handle); - -This is called during the option negotiation phase to find out what -errors to reflect to the client. An error explicitly reported to -C<nbdkit_set_error> takes precedence, but plugins written in C can -often get by with implicitly using the value stored in C<errno>, when -this callback returns true. On the other hand, a plugin written in any -other language generally cannot control the value of C<errno> after -the glue code of the language binding wrappers; returning false forces -the use of C<EIO> as the fallback error rather than using an -unreliable value in C<errno>. This callback is not needed in the -bindings of other languages. The results of this callback do not -matter if all data serving callbacks correctly call -C<nbdkit_set_error>. - -If there is an error, C<.errno_is_reliable> should call C<nbdkit_error> -with an error message and return C<-1>. - -This callback is not required. If omitted, then we return true for -a plugin written in C, and false for plugins in other languages. - =head2 C<.pread> int pread (void *handle, void *buf, uint32_t count, uint64_t offset); diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h index 568aaf5..95cba8d 100644 --- a/include/nbdkit-plugin.h +++ b/include/nbdkit-plugin.h @@ -92,7 +92,7 @@ struct nbdkit_plugin { int (*trim) (void *handle, uint32_t count, uint64_t offset); int (*zero) (void *handle, uint32_t count, uint64_t offset, int may_trim); - int (*errno_is_reliable) (void *handle); + int errno_is_preserved; /* int (*set_exportname) (void *handle, const char *exportname); */ }; diff --git a/plugins/ocaml/nbdkit-ocaml-plugin.pod b/plugins/ocaml/nbdkit-ocaml-plugin.pod index 250c1da..8f67d9b 100644 --- a/plugins/ocaml/nbdkit-ocaml-plugin.pod +++ b/plugins/ocaml/nbdkit-ocaml-plugin.pod @@ -89,17 +89,6 @@ handle struct in your plugin: printf "handle ID = %d\n" handle.h_id; (* ... *) -=head2 MISSING CALLBACKS - -=over 4 - -=item Missing: C<errno_is_reliable> - -This is not needed because the process of gluing OCaml code into C cannot -reliably use C<errno>. - -=back - =head2 THREADS The first parameter of C<NBDKit.register_plugin> is the thread model, diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c index 5d7aeeb..b38789a 100644 --- a/plugins/ocaml/ocaml.c +++ b/plugins/ocaml/ocaml.c @@ -499,16 +499,6 @@ SET(pwrite) SET(flush) SET(trim) -/* We can't guarantee that errno is stable across language binding - * glue code, so this callback is implemented in C only, and not - * exposed in OCaml. - */ -static int -plugin_ocaml_errno_is_reliable (void *handle) -{ - return 0; -} - /* We can't directly use NBDKIT_REGISTER_PLUGIN(). */ struct nbdkit_plugin * plugin_init (void) @@ -517,6 +507,5 @@ plugin_init (void) fprintf (stderr, "error: OCaml code did not call NBDKit.register_plugin\n"); exit (EXIT_FAILURE); } - plugin.errno_is_reliable = plugin_ocaml_errno_is_reliable; return &plugin; } diff --git a/plugins/perl/nbdkit-perl-plugin.pod b/plugins/perl/nbdkit-perl-plugin.pod index 920bdef..2ecdcf1 100644 --- a/plugins/perl/nbdkit-perl-plugin.pod +++ b/plugins/perl/nbdkit-perl-plugin.pod @@ -327,12 +327,6 @@ C<Nbdkit::set_error(POSIX::EOPNOTSUPP)>. These are not needed because you can just use regular Perl C<BEGIN> and C<END> constructs. -=item Missing: C<errno_is_reliable> - -This is not needed because the process of gluing Perl code into C cannot -reliably use C<errno>. Instead, call C<Nbdkit::set_error> when reporting -a failure. - =item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help> These are not yet supported. diff --git a/plugins/perl/perl.c b/plugins/perl/perl.c index 317a775..d9ad842 100644 --- a/plugins/perl/perl.c +++ b/plugins/perl/perl.c @@ -647,16 +647,6 @@ perl_trim (void *handle, uint32_t count, uint64_t offset) return 0; } -/* We can't guarantee that errno is stable across language binding - * glue code, so this callback is implemented in C only, and not - * exposed in Perl. - */ -static int -perl_errno_is_reliable (void *handle) -{ - return 0; -} - #define perl_config_help \ "script=<FILENAME> (required) The Perl plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -688,8 +678,6 @@ static struct nbdkit_plugin plugin = { .flush = perl_flush, .trim = perl_trim, .zero = perl_zero, - - .errno_is_reliable = perl_errno_is_reliable, }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index 1c57e15..90395b9 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -246,12 +246,6 @@ use C<nbdkit.set_error(errno.EOPNOTSUPP)>. These are not needed because you can just use ordinary Python constructs. -=item Missing: C<errno_is_reliable> - -This is not needed because the process of gluing Python code into C cannot -reliably use C<errno>. Instead, call C<nbdkit.set_error> when reporting -a failure. - =item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help> These are not yet supported. diff --git a/plugins/python/python.c b/plugins/python/python.c index 895a361..c4ac92d 100644 --- a/plugins/python/python.c +++ b/plugins/python/python.c @@ -612,16 +612,6 @@ py_can_trim (void *handle) return 0; } -/* We can't guarantee that errno is stable across language binding - * glue code, so this callback is implemented in C only, and not - * exposed in Python. - */ -static int -py_errno_is_reliable (void *handle) -{ - return 0; -} - #define py_config_help \ "script=<FILENAME> (required) The Python plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -653,8 +643,6 @@ static struct nbdkit_plugin plugin = { .flush = py_flush, .trim = py_trim, .zero = py_zero, - - .errno_is_reliable = py_errno_is_reliable, }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/plugins/ruby/nbdkit-ruby-plugin.pod b/plugins/ruby/nbdkit-ruby-plugin.pod index 6cf8e97..980e009 100644 --- a/plugins/ruby/nbdkit-ruby-plugin.pod +++ b/plugins/ruby/nbdkit-ruby-plugin.pod @@ -251,12 +251,6 @@ use C<Nbdkit.set_error(Errno::EOPNOTSUPP)>. These are not needed because you can just use ordinary Ruby constructs. -=item Missing: C<errno_is_reliable> - -This is not needed because the process of gluing Ruby code into C cannot -reliably use C<errno>. Instead, call C<Nbdkit.set_error> when reporting -a failure. - =item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help> These are not yet supported. diff --git a/plugins/ruby/ruby.c b/plugins/ruby/ruby.c index 33d7968..023c1e8 100644 --- a/plugins/ruby/ruby.c +++ b/plugins/ruby/ruby.c @@ -467,16 +467,6 @@ plugin_rb_can_trim (void *handle) return RTEST (rv); } -/* We can't guarantee that errno is stable across language binding - * glue code, so this callback is implemented in C only, and not - * exposed in Ruby. - */ -static int -plugin_rb_errno_is_reliable (void *handle) -{ - return 0; -} - #define plugin_rb_config_help \ "script=<FILENAME> (required) The Ruby plugin to run.\n" \ "[other arguments may be used by the plugin that you load]" @@ -511,8 +501,6 @@ static struct nbdkit_plugin plugin = { .flush = plugin_rb_flush, .trim = plugin_rb_trim, .zero = plugin_rb_zero, - - .errno_is_reliable = plugin_rb_errno_is_reliable, }; NBDKIT_REGISTER_PLUGIN(plugin) diff --git a/src/connections.c b/src/connections.c index cf06ac5..a0d689a 100644 --- a/src/connections.c +++ b/src/connections.c @@ -498,10 +498,7 @@ negotiate_handshake (struct connection *conn) int r; plugin_lock_request (conn); - conn->errno_is_reliable = plugin_errno_is_reliable (conn); - if (conn->errno_is_reliable < 0) - r = -1; - else if (!newstyle) + if (!newstyle) r = _negotiate_handshake_oldstyle (conn); else r = _negotiate_handshake_newstyle (conn); @@ -612,7 +609,7 @@ get_error (struct connection *conn) { int ret = tls_get_error (); - if (!ret && conn->errno_is_reliable) + if (!ret && plugin_errno_is_preserved ()) ret = errno; return ret ? ret : EIO; } diff --git a/src/internal.h b/src/internal.h index aa7dbd3..e73edf1 100644 --- a/src/internal.h +++ b/src/internal.h @@ -120,7 +120,6 @@ struct connection { int can_flush; int is_rotational; int can_trim; - int errno_is_reliable; }; extern int handle_single_connection (int sockin, int sockout); @@ -141,10 +140,10 @@ extern void plugin_lock_connection (void); extern void plugin_unlock_connection (void); extern void plugin_lock_request (struct connection *conn); extern void plugin_unlock_request (struct connection *conn); +extern int plugin_errno_is_preserved (void); extern int plugin_open (struct connection *conn, int readonly); extern void plugin_close (struct connection *conn); extern int64_t plugin_get_size (struct connection *conn); -extern int plugin_errno_is_reliable (struct connection *conn); extern int plugin_can_write (struct connection *conn); extern int plugin_can_flush (struct connection *conn); extern int plugin_is_rotational (struct connection *conn); diff --git a/src/plugins.c b/src/plugins.c index 837a54f..eeed8a9 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -236,6 +236,7 @@ plugin_dump_fields (void) break; } printf ("\n"); + printf ("errno_is_preserved=%d\n", plugin.errno_is_preserved); #define HAS(field) if (plugin.field) printf ("has_%s=1\n", #field) HAS (longname); @@ -350,6 +351,14 @@ plugin_unlock_request (struct connection *conn) } int +plugin_errno_is_preserved (void) +{ + assert (dl); + + return plugin.errno_is_preserved; +} + +int plugin_open (struct connection *conn, int readonly) { void *handle; @@ -395,21 +404,6 @@ plugin_get_size (struct connection *conn) } int -plugin_errno_is_reliable (struct connection *conn) -{ - assert (dl); - assert (conn->handle); - - debug ("errno_is_reliable"); - - if (plugin.errno_is_reliable) - return plugin.errno_is_reliable (conn->handle); - - /* Default to 1, for backwards compatibility (correct for C plugins) */ - return 1; -} - -int plugin_can_write (struct connection *conn) { assert (dl); @@ -548,7 +542,7 @@ plugin_zero (struct connection *conn, result = plugin.zero (conn->handle, count, offset, may_trim); if (result == -1) { err = tls_get_error (); - if (!err && conn->errno_is_reliable) + if (!err && plugin_errno_is_preserved ()) err = errno; } if (result == 0 || err != EOPNOTSUPP) -- 2.10.2