Breaking Changes to Imageflow ABI/FFI interface:

* Modify signature of imageflow_io_create_for_file(context, mode, filename) -> * imageflow_job_io (Remove imageflow_cleanup_with parameter)
* Modify signature of imageflow_io_create_from_buffer(context, buffer, buffer_byte_coutn, lifteime) (Remove imageflow_cleanup_with parameter)
* Remove CleanupWith enumeration (Just don't reuse contexts much)
* Mark  imageflow_context_error_code(context) as unstabilized (don't depend on this)
* Remove imageflow_context_raise_error (no usage scenario)
* Remove imageflow_context_add_to_callstack (no usage scenario)
* Remove imageflow_context_clear_error(context) -> void (replaced with try_clear)

Improvements to ABI
* Add panic recovery to all methods
* Add null-parameter checking to all methods
* And most-significant-bit checking (for size_t) to all methods
* Add imageflow_context_error_as_exit_code(context) -> i32 (and stabilize values)
* Add imageflow_context_error_as_http_code(context) -> 32 (and stabilize values)
* Add imageflow_context_error_try_clear(context) -> bool
* Add imageflow_context_error_recoverable(context) -> bool

Changes to c_components
* add flow_context_set_error_get_message_buffer_info and flow_context_error_status_included_in_message(c) -> bool
* Remove status codes 70,71,72,73
* Add status code 90 (Error reporting inconsistency)
* Modify error message serialization (prepend with 'CError %d: [status description]: ', unless status_included_in_message=true - in which case prepend nothing)
* Reduce callstack_capacity from 14 to 8
* Move flow_error_info to bottom of flow_context, to improve data locality

Continue refactoring codebase to improve error handling and clarity

README.md

@@ -137,6 +137,10 @@ sudo pip install conan
 ./ci/nixtools/install_dssim.sh
 ./build.sh
 ```
+
+The build will fail with `settings.target_cpu`. Edit `$HOME\.conan\settings.yml`. Append the line `target_cpu: [x86, x86-64, nehalem, sandybridge, haswell, native]`
+
+
 We aren't listing dependencies needed for 
 
 * Valgrind (common versions break openssl; you may need to build from source)
@@ -155,6 +159,9 @@ brew install conan nasm cmake
 ./build.sh
 ```
 
+The build will fail with `settings.target_cpu`. Edit `$HOME\.conan\settings.yml`. Append the line `target_cpu: [x86, x86-64, nehalem, sandybridge, haswell, native]`
+
+
 ## Windows
 
 Don't use a C++ IDE until you've run `win_build_c.bat`, as CMake needs to generate files.

c_components/imageflow.h

@@ -83,11 +83,9 @@ typedef enum flow_status_code {
 
     flow_status_Image_decoding_failed = 60,
     flow_status_Image_encoding_failed = 61,
-    flow_status_Graph_invalid = 70,
-    flow_status_Graph_is_cyclic = 71,
-    flow_status_Invalid_inputs_to_node = 72,
-    flow_status_Maximum_graph_passes_exceeded = 73,
     flow_status_ErrorReportingInconsistency = 90,
+    flow_status_First_rust_error = 200,
+
     flow_status_Other_error = 1024,
     flow_status____Last_library_error,
     flow_status_First_user_defined_error = 1025,
@@ -194,6 +192,8 @@ PUB void flow_context_terminate(flow_c * context);
 
 PUB void flow_context_destroy(flow_c * c); // Don't pass this a pointer on the stack! use begin/end terminate
 
+PUB bool flow_context_error_status_included_in_message(flow_c * context); //Useful for roundtripping
+PUB bool flow_context_set_error_get_message_buffer_info(flow_c * context, flow_status_code code, bool status_included_in_buffer, char * * buffer,  size_t * buffer_size);
 PUB int64_t flow_context_error_and_stacktrace(flow_c * c, char * buffer, size_t buffer_size, bool full_file_path);
 PUB int64_t flow_context_error_message(flow_c * c, char * buffer, size_t buffer_size);
 PUB int64_t flow_context_stacktrace(flow_c * c, char * buffer, size_t buffer_size, bool full_file_path);

c_components/lib/bitmap_formats.c

@@ -256,7 +256,7 @@ bool flow_bitmap_bgra_fill_rect(flow_c * c, struct flow_bitmap_bgra * b, uint32_
     uint32_t color = color_srgb_argb;
     if (step == 1) {
         // TODO: use gamma-correct grayscale conversion
-        FLOW_error(c, flow_status_Not_implemented);
+        FLOW_error(c, flow_status_Unsupported_pixel_format);
         return false;
     } else if (step == 3) {
         // we just ignore the alpha bits

c_components/lib/context.c

@@ -62,6 +62,29 @@ char * flow_context_set_error_get_message_buffer(flow_c * context, flow_status_c
     return &context->error.message[0];
 }
 
+// Returns true if the operation succeeded
+// Does not add to call stack
+bool flow_context_set_error_get_message_buffer_info(flow_c * context, flow_status_code code, bool status_included_in_buffer, char * * buffer,  size_t * buffer_size)
+{
+    if (context->error.reason != flow_status_No_Error) {
+        // The last error wasn't cleared, lock it down. We prefer the original error.
+        context->error.locked = true;
+        *buffer = 0;
+        *buffer_size = 0;
+        return false;
+    }else {
+        context->error.status_included_in_message = status_included_in_buffer;
+        if (code == flow_status_No_Error) {
+            context->error.reason = flow_status_Other_error;
+        } else {
+            context->error.reason = code;
+        }
+        *buffer = &context->error.message[0];
+        *buffer_size = FLOW_ERROR_MESSAGE_SIZE;
+        return true;
+    }
+}
+
 bool flow_context_add_to_callstack(flow_c * context, const char * file, int line, const char * function_name)
 {
     if (context->error.callstack_count < context->error.callstack_capacity && !context->error.locked
@@ -70,6 +93,7 @@ bool flow_context_add_to_callstack(flow_c * context, const char * file, int line
         context->error.callstack[context->error.callstack_count].line = line;
         context->error.callstack[context->error.callstack_count].function_name = function_name;
         context->error.callstack_count++;
+        context->error.status_included_in_message = false;
         return true;
     }
     return false;
@@ -83,6 +107,7 @@ void flow_context_clear_error(flow_c * context)
     context->error.callstack[0].function_name = NULL;
     context->error.reason = flow_status_No_Error;
     context->error.locked = false;
+    context->error.status_included_in_message = false;
     context->error.message[0] = 0;
 }
 
@@ -121,20 +146,16 @@ static const char * status_code_to_string(flow_status_code code)
             return "Image decoding failed";
         case 61:
             return "Image encoding failed";
-        case 70:
-            return "Graph invalid";
-        case 71:
-            return "Graph is cyclic";
-        case 72:
-            return "Invalid inputs to node";
-        case 73:
-            return "Maximum graph passes exceeded";
         case 90:
             return "C Error Reporting Inconsistency";
         case 1024:
             return "Other error";
         default:
-            return "Unknown status code";
+            if (code >= flow_status_First_rust_error && code < flow_status_Other_error) {
+                return "Rust status code";
+            }else {
+                return "Unknown status code";
+            }
     }
 }
 
@@ -188,15 +209,34 @@ int64_t flow_context_error_and_stacktrace(flow_c * context, char * buffer, size_
 
     return original_buffer_size - buffer_size;
 }
+bool flow_context_error_status_included_in_message(flow_c * context){
+    return context->error.status_included_in_message;
+}
 
 int64_t flow_context_error_message(flow_c * context, char * buffer, size_t buffer_size)
 {
     int chars_written = 0;
-    if (context->error.message[0] == 0) {
-        chars_written = flow_snprintf(buffer, buffer_size, "%s", status_code_to_string(context->error.reason));
-    } else {
-        chars_written = flow_snprintf(buffer, buffer_size, "%s : %s", status_code_to_string(context->error.reason),
-                                      context->error.message);
+    const char * reason_str = status_code_to_string(context->error.reason);
+    if (context->error.reason == flow_status_No_Error){
+        chars_written = flow_snprintf(buffer, buffer_size, "%s", reason_str);
+    }else {
+        if (context->error.status_included_in_message == true) {
+            if (context->error.message[0] == 0) {
+                // This branch shouldn't happen
+                chars_written = flow_snprintf(buffer, buffer_size, "CError of Rust Error %d - message missing",
+                                              (int)context->error.reason - 200);
+            } else {
+                chars_written = flow_snprintf(buffer, buffer_size, "%s", context->error.message);
+            }
+        } else {
+            if (context->error.message[0] == 0) {
+                chars_written = flow_snprintf(buffer, buffer_size, "CError %d: %s", context->error.reason, reason_str);
+            } else {
+                chars_written = flow_snprintf(buffer, buffer_size, "CError %d: %s : %s", context->error.reason,
+                                              reason_str,
+                                              context->error.message);
+            }
+        }
     }
     if (chars_written < 0) {
         return -1; // we ran out of space
@@ -246,7 +286,7 @@ void flow_context_initialize(flow_c * context)
     context->log.log = NULL;
     context->log.capacity = 0;
     context->log.count = 0;
-    context->error.callstack_capacity = 14;
+    context->error.callstack_capacity = 8;
     context->error.callstack_count = 0;
     context->error.callstack[0].file = NULL;
     context->error.callstack[0].line = -1;

c_components/lib/imageflow_private.h

@@ -89,10 +89,11 @@ struct flow_error_callstack_line {
 
 struct flow_error_info {
     flow_status_code reason;
-    struct flow_error_callstack_line callstack[14];
+    struct flow_error_callstack_line callstack[8];
     int callstack_count;
     int callstack_capacity;
     bool locked;
+    bool status_included_in_message;
     char message[FLOW_ERROR_MESSAGE_SIZE + 1];
 };
 
@@ -148,11 +149,11 @@ struct flow_objtracking_info {
 /** flow_context: main structure **/
 
 struct flow_context {
-    struct flow_error_info error;
+    struct flow_context_codec_set * codec_set;
     struct flow_heap underlying_heap;
-    struct flow_profiling_log log;
     struct flow_objtracking_info object_tracking;
-    struct flow_context_codec_set * codec_set;
+    struct flow_profiling_log log;
+    struct flow_error_info error;
 };
 
 typedef struct flow_context flow_c;

c_components/profile.sh

@@ -2,15 +2,17 @@
 set -e
 
 # Delete the profiling dir to re-install
+rm -rf profiling
 (
 if [ -d "profiling" ]; then
     cd profiling || exit
 else
     mkdir profiling
     cd profiling || exit
-    conan install --file ../conanfile.py --scope profiling=True --scope build_tests=False --build missing
+    conan install --scope profiling=True --scope build_tests=False -s target_cpu=haswell --build missing -u ../
 fi
-conan build --file ../conanfile.py
+conan build ../
+
 time build/bin/profile_imageflow
 gprof build/bin/profile_imageflow gmon.out > ../profile.txt
 )

c_components/tests/test_context.cpp

@@ -10,6 +10,13 @@ TEST_CASE("Verify .cpp and .c files are being compiled with compatible type size
     REQUIRE(info.sizeof_size_t == sizeof(size_t));
     REQUIRE(info.sizeof_int == sizeof(int));
 }
+
+TEST_CASE("Test size of flow_context")
+{
+    printf("sizeof(flow_c) = %lu", sizeof(flow_c));
+    REQUIRE(sizeof(flow_c) < 1500);
+}
+
 TEST_CASE("Test flow_snprintf with single-character buffer", "")
 {
     char buf[] = { 3, 25 };
@@ -62,7 +69,7 @@ TEST_CASE("Test error message printing", "")
     char buf[4096];
     int64_t chars_written = flow_context_error_and_stacktrace(c, buf, 4096, false);
     REQUIRE(chars_written > 0);
-    REQUIRE_THAT(buf, StartsWith("Invalid argument : You passed a value outside [0,1]: 3\ntest_context.cpp:"));
+    REQUIRE_THAT(buf, StartsWith("CError 50: Invalid argument : You passed a value outside [0,1]: 3\ntest_context.cpp:"));
 
     flow_context_destroy(c);
 }
@@ -77,7 +84,7 @@ TEST_CASE("Test error message printing with null files or functions in the stack
     char buf[4096];
     int64_t chars_written = flow_context_error_and_stacktrace(c, buf, 4096, false);
     REQUIRE(chars_written > 0);
-    REQUIRE(buf == std::string("Invalid argument\n(unknown):25: in function (unknown)\n"));
+    REQUIRE(buf == std::string("CError 50: Invalid argument\n(unknown):25: in function (unknown)\n"));
 
     flow_context_destroy(c);
 }
@@ -98,7 +105,7 @@ TEST_CASE("Test reporting of a failing destructor", "")
     char buf[4096];
     int64_t chars_written = flow_context_error_and_stacktrace(c, buf, 4096, false);
     REQUIRE(chars_written > 0);
-    REQUIRE_THAT(buf, StartsWith("Other error : Destructor returned false, indicating failure"));
+    REQUIRE_THAT(buf, StartsWith("CError 1024: Other error : Destructor returned false, indicating failure"));
 
     flow_context_destroy(c);
 }

imageflow_abi/Cargo.toml

@@ -14,6 +14,8 @@ doctest = false
 [dependencies]
 libc = "0.2"
 imageflow_core = { path = "../imageflow_core", version = "*" }
+backtrace = "*"
+smallvec="*"
 
 [build-dependencies]
 imageflow_helpers = { path = "../imageflow_helpers", version = "*" }