Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robust Error Handling #12

Open
esshka opened this issue Jun 15, 2024 · 0 comments
Open

Robust Error Handling #12

esshka opened this issue Jun 15, 2024 · 0 comments

Comments

@esshka
Copy link

esshka commented Jun 15, 2024

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);
 }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant