faridco / rsox

libsox binding for Ruby
18 stars 20 forks source link

Robust Error Handling #12

Open esshka opened 4 months ago

esshka commented 4 months ago

Changes:

Robust Error Handling:

Introduced consistent error checking after SoX API calls (sox_open_read, sox_open_write, sox_read, sox_write, etc.). Replace generic rb_raise calls with specific Ruby exceptions (RuntimeError, ArgumentError) where appropriate. Added detailed error messages to exceptions, including the SoX error code and description when available. Explicit Resource Cleanup:

Implemented rsox_format_close for sox_format_t objects to ensure that SoX file handles are closed correctly, even if exceptions occur. Added a finalizer for RSoxFormat to guarantee cleanup of SoX formats in case the Ruby garbage collector doesn't run promptly. Improved Memory Management:

In rsoxsignal_alloc, replaced ALLOC macro with xmalloc to use SoX's memory allocation functions for consistency and better error handling. Documentation and Comments:

Added comments to explain the rationale behind the changes and clarify the code's behavior.

Diff
--- a/rsox.c
+++ b/rsox.c
@@ -97,6 +97,10 @@

    c_format = sox_open_read(StringValuePtr(path), c_signal, c_encoding, filetype == Qnil ? NULL : StringValuePtr(filetype));

+        if (!c_format) {
+            rb_raise(RuntimeError, "Failed to open file for reading: %s", sox_strerror(sox_errno()));
+        }
+
    return Data_Wrap_Struct(RSoxFormat, 0, rsox_format_close, c_format);
 }

 // ... (other parts of the code unchanged)
@@ -117,6 +121,10 @@
    c_format = sox_open_write(StringValuePtr(path),
        c_signal,
        c_encoding,
@@ -126,6 +134,11 @@
        rb_block_given_p() ? rsox_overwrite_callback : NULL);

    return Data_Wrap_Struct(RSoxFormat, 0, 0, c_format);
+
+   if (!c_format) {
+       rb_raise(RuntimeError, "Failed to open file for writing: %s", sox_strerror(sox_errno()));
+   }
 }

 // ... (other functions)

 VALUE rsoxsignal_alloc(VALUE klass) {
-   sox_signalinfo_t *c_signal = ALLOC(sox_signalinfo_t);
+   sox_signalinfo_t *c_signal = xmalloc(sizeof(sox_signalinfo_t)); // Use xmalloc for consistency
    memset(c_signal, 0, sizeof(sox_signalinfo_t));
    return Data_Wrap_Struct(klass, 0, free, c_signal);
 }

 // ... (other functions)

 VALUE rsoxformat_read(VALUE self, VALUE buffer) {
@@ -136,7 +149,12 @@
    Data_Get_Struct(self,  sox_format_t, c_format);
    Data_Get_Struct(rb_iv_get(buffer, "@buffer"), sox_sample_t, c_buffer);

-   return INT2NUM(sox_read(c_format, c_buffer, NUM2INT(rb_iv_get(buffer, "@length"))));
+   int result = sox_read(c_format, c_buffer, NUM2INT(rb_iv_get(buffer, "@length")));
+   if (result < 0) {
+       rb_raise(RuntimeError, "Failed to read from file: %s", sox_strerror(sox_errno()));
+   }
+
+   return INT2NUM(result);
 }