Rewrite to be safer

Author: jeroen

NEWS

@@ -1,2 +1,3 @@
-0.8.3.1
+0.8.6
   - save_gif() now properly closes graphics device when expr errors
+  - safer C code to prevent segfaults on write errors

src/Makevars.in

@@ -4,7 +4,7 @@ PKG_CFLAGS = -pthread
 PKG_LIBS = -L$(LIBDIR) -lmyrustlib -lresolv -pthread
 CARGO = @cargobin@
 
-all: clean
+#all: clean
 
 $(SHLIB): $(STATLIB)
 

src/wrapper.c

@@ -8,18 +8,15 @@
 
 /* data to pass to encoder thread */
 typedef struct {
-  int complete;
   const char * path;
+  GifskiError err;
   gifski * g;
 } gifski_encoder_thread_info;
 
 /* gifski_write() blocks until main thread calls gifski_end_adding_frames() */
 void * gifski_encoder_thread(void * data){
   gifski_encoder_thread_info * info = data;
-  if(gifski_write(info->g, info->path) != GIFSKI_OK)
-    REprintf("Failure writing to '%s'\n", info->path);
-  gifski_drop(info->g);
-  info->complete = 1;
+  info->err = gifski_write(info->g, info->path);
   return NULL;
 }
 
@@ -39,20 +36,18 @@ SEXP R_png_to_gif(SEXP png_files, SEXP gif_file, SEXP width, SEXP height, SEXP d
 
   /* create encoder thread; TODO: maybe we can use multiple encoder threads? */
   pthread_t encoder_thread;
-  gifski_encoder_thread_info info = {0, CHAR(STRING_ELT(gif_file, 0)), g};
+  gifski_encoder_thread_info info = {CHAR(STRING_ELT(gif_file, 0)), GIFSKI_OK, g};
   if(pthread_create(&encoder_thread, NULL, gifski_encoder_thread, &info))
     Rf_error("Failed to create encoder thread");
 
   /* add png frames in main thread */
   for(size_t i = 0; i < Rf_length(png_files); i++){
-    if(info.complete){
-      gifski_drop(g);
-      Rf_error("Writer thread has died");
-    }
+    if(info.err != GIFSKI_OK)
+      goto cleanup;
     if(gifski_add_frame_png_file(g, i, CHAR(STRING_ELT(png_files, i)), Rf_asInteger(delay)) != GIFSKI_OK)
       Rprintf("Failed to add frame %d\n", i);
     if(Rf_asLogical(progress))
-      Rprintf("\rFrame %d (%d%%)", i, (i+1) * 100 / Rf_length(png_files));
+      Rprintf("\rFrame %d (%d%%)", (i+1), (i+1) * 100 / Rf_length(png_files));
   }
 
   /* This will finalize the encoder thread as well */
@@ -65,6 +60,11 @@ SEXP R_png_to_gif(SEXP png_files, SEXP gif_file, SEXP width, SEXP height, SEXP d
   /* wait for the encoder thread to finish */
   if(pthread_join(encoder_thread, NULL))
     Rf_error("Failed to join encoder_thread");
+
+cleanup:
+  gifski_drop(g);
+  if(info.err != GIFSKI_OK)
+    Rf_error("Failure writing image %s", info.path);
   return gif_file;
 }