From cbf01be4126cbc0f6d80364a7e0b6ad777a7d8ae Mon Sep 17 00:00:00 2001 From: David Robillard Date: Sat, 9 Oct 2021 13:44:31 -0400 Subject: Fix handling of deferred write errors that happen when closing --- include/serd/serd.h | 12 ++++++++---- src/output_stream.c | 27 ++++++++++++++------------- test/meson.build | 6 ++++++ test/test_deferred_write_error.py | 31 +++++++++++++++++++++++++++++++ test/test_model.c | 4 ++-- test/test_writer.c | 15 +++++++++------ tools/console.c | 6 ++++-- tools/serd-filter.c | 2 +- tools/serd-pipe.c | 2 +- tools/serd-sort.c | 2 +- 10 files changed, 77 insertions(+), 30 deletions(-) create mode 100644 test/test_deferred_write_error.py diff --git a/include/serd/serd.h b/include/serd/serd.h index d606f4bb..35d31a57 100644 --- a/include/serd/serd.h +++ b/include/serd/serd.h @@ -2201,8 +2201,8 @@ serd_node_to_syntax(const SerdNode* SERD_NONNULL node, /// An input stream that produces bytes typedef struct { void* SERD_NULLABLE stream; ///< Opaque parameter for functions - SerdReadFunc SERD_NULLABLE read; ///< Read bytes to input - SerdErrorFunc SERD_NONNULL error; ///< Stream error accessor + SerdReadFunc SERD_NONNULL read; ///< Read bytes from input + SerdErrorFunc SERD_NULLABLE error; ///< Stream error accessor SerdCloseFunc SERD_NULLABLE close; ///< Close input } SerdInputStream; @@ -2468,7 +2468,8 @@ serd_buffer_close(void* SERD_NONNULL stream); /// An output stream that receives bytes typedef struct { void* SERD_NULLABLE stream; ///< Opaque parameter for functions - SerdWriteFunc SERD_NULLABLE write; ///< Write bytes to output + SerdWriteFunc SERD_NONNULL write; ///< Write bytes to output + SerdErrorFunc SERD_NULLABLE error; ///< Stream error accessor SerdCloseFunc SERD_NULLABLE close; ///< Close output } SerdOutputStream; @@ -2476,6 +2477,7 @@ typedef struct { Open a stream that writes to a provided function. @param write_func Function to write output. + @param error_func Function used to detect errors. @param close_func Function to close the stream after writing is done. @param stream Opaque stream parameter for write_func and close_func. @@ -2484,6 +2486,7 @@ typedef struct { SERD_CONST_API SerdOutputStream serd_open_output_stream(SerdWriteFunc SERD_NONNULL write_func, + SerdErrorFunc SERD_NULLABLE error_func, SerdCloseFunc SERD_NULLABLE close_func, void* SERD_NULLABLE stream); @@ -2519,7 +2522,8 @@ serd_open_output_file(const char* SERD_NONNULL path); This will call the close function, and reset the stream internally so that no further writes can be made. For convenience, this is safe to call on - NULL, and safe to call several times on the same output. + NULL, and safe to call several times on the same output. Failure is + returned in both of those cases. */ SERD_API SerdStatus diff --git a/src/output_stream.c b/src/output_stream.c index b98c834a..54397735 100644 --- a/src/output_stream.c +++ b/src/output_stream.c @@ -21,6 +21,7 @@ // IWYU pragma: no_include #include +#include #include #include @@ -30,12 +31,13 @@ SerdOutputStream serd_open_output_stream(SerdWriteFunc const write_func, + SerdErrorFunc const error_func, SerdCloseFunc const close_func, void* const stream) { assert(write_func); - SerdOutputStream output = {stream, write_func, close_func}; + SerdOutputStream output = {stream, write_func, error_func, close_func}; return output; } @@ -44,7 +46,8 @@ serd_open_output_buffer(SerdBuffer* const buffer) { assert(buffer); - return serd_open_output_stream(serd_buffer_write, serd_buffer_close, buffer); + return serd_open_output_stream( + serd_buffer_write, NULL, serd_buffer_close, buffer); } SerdOutputStream @@ -59,7 +62,7 @@ serd_open_output_file(const char* const path) #endif if (!file) { - const SerdOutputStream failure = {NULL, NULL, NULL}; + const SerdOutputStream failure = {NULL, NULL, NULL, NULL}; return failure; } @@ -68,22 +71,20 @@ serd_open_output_file(const char* const path) #endif return serd_open_output_stream( - (SerdWriteFunc)fwrite, (SerdCloseFunc)fclose, file); + (SerdWriteFunc)fwrite, (SerdErrorFunc)ferror, (SerdCloseFunc)fclose, file); } SerdStatus serd_close_output(SerdOutputStream* const output) { - int ret = 0; + if (!output || !output->stream) { + return SERD_FAILURE; + } - if (output) { - if (output->close && output->stream) { - ret = output->close(output->stream); - output->stream = NULL; - } + const bool had_error = output->error ? output->error(output->stream) : false; + int close_st = output->close ? output->close(output->stream) : 0; - output->stream = NULL; - } + output->stream = NULL; - return ret ? SERD_BAD_WRITE : SERD_SUCCESS; + return (had_error || close_st) ? SERD_BAD_WRITE : SERD_SUCCESS; } diff --git a/test/meson.build b/test/meson.build index ae66ba37..a25ac6de 100644 --- a/test/meson.build +++ b/test/meson.build @@ -52,6 +52,7 @@ if autoship.found() endif serd_ttl = files('../serd.ttl')[0] +small_ttl = files('../test/TurtleTests/turtle-syntax-number-01.ttl')[0] common_script_options = [] if wrapper != '' common_script_options = ['--wrapper', wrapper] @@ -230,6 +231,11 @@ if is_variable('serd_pipe') env: test_env, suite: ['tools', 'pipe', 'output']) + test('deferred_write_error', files('test_deferred_write_error.py'), + args: pipe_test_script_args + [small_ttl], + env: test_env, + suite: ['tools', 'pipe', 'output']) + test('missing_output', tool, args: ['-o', '/does/not/exist.ttl', meson.source_root() / 'serd.ttl'], diff --git a/test/test_deferred_write_error.py b/test/test_deferred_write_error.py new file mode 100644 index 00000000..4005d570 --- /dev/null +++ b/test/test_deferred_write_error.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 + +"""Test errors that only really happen when the output file is closed.""" + +import argparse +import sys +import shlex +import subprocess +import os + +parser = argparse.ArgumentParser(description=__doc__) + +parser.add_argument("--tool", default="tools/serd-pipe", help="executable") +parser.add_argument("--wrapper", default="", help="executable wrapper") +parser.add_argument("input", help="valid input file") + +args = parser.parse_args(sys.argv[1:]) +command = shlex.split(args.wrapper) + [args.tool, '-b', '1024', args.input] + +if os.path.exists("/dev/full"): + + with open("/dev/full", "w") as out: + proc = subprocess.run( + command, check=False, stdout=out, stderr=subprocess.PIPE + ) + + assert proc.returncode != 0 + assert "error" in proc.stderr.decode("utf-8") + +else: + sys.stderr.write("warning: /dev/full not present, skipping test") diff --git a/test/test_model.c b/test/test_model.c index 3e781c7e..a64a7b7e 100644 --- a/test/test_model.c +++ b/test/test_model.c @@ -1281,7 +1281,7 @@ test_write_error_in_list_subject(SerdWorld* world, const unsigned n_quads) for (size_t max_successes = 0; max_successes < 18; ++max_successes) { FailingWriteFuncState state = {0, max_successes}; SerdOutputStream out = - serd_open_output_stream(failing_write_func, NULL, &state); + serd_open_output_stream(failing_write_func, NULL, NULL, &state); SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0, env, &out, 1); @@ -1337,7 +1337,7 @@ test_write_error_in_list_object(SerdWorld* world, const unsigned n_quads) for (size_t max_successes = 0; max_successes < 21; ++max_successes) { FailingWriteFuncState state = {0, max_successes}; SerdOutputStream out = - serd_open_output_stream(failing_write_func, NULL, &state); + serd_open_output_stream(failing_write_func, NULL, NULL, &state); SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0, env, &out, 1); diff --git a/test/test_writer.c b/test/test_writer.c index c719642f..6ffe5d0f 100644 --- a/test/test_writer.c +++ b/test/test_writer.c @@ -126,10 +126,12 @@ null_sink(const void* const buf, static void test_writer_stack_overflow(void) { - SerdWorld* world = serd_world_new(); - SerdNodes* nodes = serd_world_nodes(world); - SerdEnv* env = serd_env_new(SERD_EMPTY_STRING()); - SerdOutputStream output = serd_open_output_stream(null_sink, NULL, NULL); + SerdWorld* world = serd_world_new(); + SerdNodes* nodes = serd_world_nodes(world); + SerdEnv* env = serd_env_new(SERD_EMPTY_STRING()); + + SerdOutputStream output = + serd_open_output_stream(null_sink, NULL, NULL, NULL); SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0u, env, &output, 1); @@ -244,7 +246,8 @@ test_write_error(void) // Test with setting errno - SerdOutputStream output = serd_open_output_stream(faulty_sink, NULL, NULL); + SerdOutputStream output = + serd_open_output_stream(faulty_sink, NULL, NULL, NULL); SerdWriter* writer = serd_writer_new(world, SERD_TURTLE, 0u, env, &output, 1); assert(writer); @@ -256,7 +259,7 @@ test_write_error(void) serd_close_output(&output); // Test without setting errno - output = serd_open_output_stream(faulty_sink, NULL, world); + output = serd_open_output_stream(faulty_sink, NULL, NULL, world); writer = serd_writer_new(world, SERD_TURTLE, 0u, env, &output, 1); assert(writer); diff --git a/tools/console.c b/tools/console.c index 3b209273..c6e55c17 100644 --- a/tools/console.c +++ b/tools/console.c @@ -372,8 +372,10 @@ serd_open_tool_output(const char* const filename) { if (!filename || !strcmp(filename, "-")) { serd_set_stream_utf8_mode(stdout); - return serd_open_output_stream( - (SerdWriteFunc)fwrite, (SerdCloseFunc)fclose, stdout); + return serd_open_output_stream((SerdWriteFunc)fwrite, + (SerdErrorFunc)ferror, + (SerdCloseFunc)fclose, + stdout); } return serd_open_output_file(filename); diff --git a/tools/serd-filter.c b/tools/serd-filter.c index 0804d6ba..b172d05d 100644 --- a/tools/serd-filter.c +++ b/tools/serd-filter.c @@ -127,7 +127,7 @@ log_error(SerdWorld* const world, const char* const fmt, ...) static SerdStatus run(Options opts) { - SerdTool app = {{NULL, NULL, NULL}, NULL, NULL, NULL}; + SerdTool app = {{NULL, NULL, NULL, NULL}, NULL, NULL, NULL}; // Set up the writing environment SerdStatus st = SERD_SUCCESS; diff --git a/tools/serd-pipe.c b/tools/serd-pipe.c index f2891f9b..612826ca 100644 --- a/tools/serd-pipe.c +++ b/tools/serd-pipe.c @@ -39,7 +39,7 @@ typedef struct { static SerdStatus run(const Options opts) { - SerdTool app = {{NULL, NULL, NULL}, NULL, NULL, NULL}; + SerdTool app = {{NULL, NULL, NULL, NULL}, NULL, NULL, NULL}; // Set up the writing environment SerdStatus st = SERD_SUCCESS; diff --git a/tools/serd-sort.c b/tools/serd-sort.c index 7219fa26..267e75f8 100644 --- a/tools/serd-sort.c +++ b/tools/serd-sort.c @@ -57,7 +57,7 @@ input_has_graphs(const Options opts) static SerdStatus run(const Options opts) { - SerdTool app = {{NULL, NULL, NULL}, NULL, NULL, NULL}; + SerdTool app = {{NULL, NULL, NULL, NULL}, NULL, NULL, NULL}; // Set up the writing environment SerdStatus st = SERD_SUCCESS; -- cgit v1.2.1