rsox icon indicating copy to clipboard operation
rsox copied to clipboard

Robust Error Handling

Open esshka opened this issue 8 months ago • 0 comments

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);
 }

esshka avatar Jun 15 '24 20:06 esshka