Merge pull request #2213 from mgreter/bugfix/ctx-options-leak

Fix some minor memory leaks with context options

Author: mgreter

docs/api-context.md

@@ -174,6 +174,7 @@ int sass_compiler_execute (struct Sass_Compiler* compiler);
 // Release all memory allocated with the compiler
 // This does _not_ include any contexts or options
 void sass_delete_compiler (struct Sass_Compiler* compiler);
+void sass_delete_options(struct Sass_Options* options);
 
 // Release all memory allocated and also ourself
 void sass_delete_file_context (struct Sass_File_Context* ctx);

include/sass/context.h

@@ -50,6 +50,7 @@ ADDAPI int ADDCALL sass_compiler_execute(struct Sass_Compiler* compiler);
 // Release all memory allocated with the compiler
 // This does _not_ include any contexts or options
 ADDAPI void ADDCALL sass_delete_compiler(struct Sass_Compiler* compiler);
+ADDAPI void ADDCALL sass_delete_options(struct Sass_Options* options);
 
 // Release all memory allocated and also ourself
 ADDAPI void ADDCALL sass_delete_file_context (struct Sass_File_Context* ctx);

src/expand.cpp

@@ -141,10 +141,9 @@ namespace Sass {
     }
 
     selector_stack.push_back(sel);
-    Env* env = 0;
+    Env env(environment());
     if (block_stack.back()->is_root()) {
-      env = new Env(environment());
-      env_stack.push_back(env);
+      env_stack.push_back(&env);
     }
     sel->set_media_block(media_block_stack.back());
     Block* blk = r->block()->perform(this)->block();
@@ -155,8 +154,8 @@ namespace Sass {
     selector_stack.pop_back();
     if (block_stack.back()->is_root()) {
       env_stack.pop_back();
-      delete env;
     }
+
     rr->is_root(r->is_root());
     rr->tabs(r->tabs());
 

src/json.cpp

@@ -399,7 +399,13 @@ char *json_encode_string(const char *str)
   SB sb;
   sb_init(&sb);
 
-  emit_string(&sb, str);
+  try {
+    emit_string(&sb, str);
+  }
+  catch (std::exception &e) {
+    sb_free(&sb);
+    throw;
+  }
 
   return sb_finish(&sb);
 }
@@ -409,10 +415,16 @@ char *json_stringify(const JsonNode *node, const char *space)
   SB sb;
   sb_init(&sb);
 
-  if (space != NULL)
-    emit_value_indented(&sb, node, space, 0);
-  else
-    emit_value(&sb, node);
+  try {
+    if (space != NULL)
+      emit_value_indented(&sb, node, space, 0);
+    else
+      emit_value(&sb, node);
+  }
+  catch (std::exception &e) {
+    sb_free(&sb);
+    throw;
+  }
 
   return sb_finish(&sb);
 }

src/sass_context.cpp

@@ -31,7 +31,16 @@ namespace Sass {
 extern "C" {
   using namespace Sass;
 
-  static void copy_options(struct Sass_Options* to, struct Sass_Options* from) { *to = *from; }
+  static void sass_clear_options (struct Sass_Options* options);
+  static void sass_reset_options (struct Sass_Options* options);
+  static void copy_options(struct Sass_Options* to, struct Sass_Options* from) {
+    // free assigned memory
+    sass_clear_options(to);
+    // move memory
+    *to = *from;
+    // Reset pointers on source
+    sass_reset_options(from);
+  }
 
   #define IMPLEMENT_SASS_OPTION_ACCESSOR(type, option) \
     type ADDCALL sass_option_get_##option (struct Sass_Options* options) { return options->option; } \
@@ -475,6 +484,24 @@ extern "C" {
     return 0;
   }
 
+  // helper function, not exported, only accessible locally
+  static void sass_reset_options (struct Sass_Options* options)
+  {
+    // free pointer before
+    // or copy/move them
+    options->input_path = 0;
+    options->output_path = 0;
+    options->plugin_path = 0;
+    options->include_path = 0;
+    options->source_map_file = 0;
+    options->source_map_root = 0;
+    options->c_functions = 0;
+    options->c_importers = 0;
+    options->c_headers = 0;
+    options->plugin_paths = 0;
+    options->include_paths = 0;
+  }
+
   // helper function, not exported, only accessible locally
   static void sass_clear_options (struct Sass_Options* options)
   {
@@ -527,12 +554,25 @@ extern "C" {
         cur = next;
       }
     }
+    // Free options strings
+    free(options->input_path);
+    free(options->output_path);
+    free(options->plugin_path);
+    free(options->include_path);
+    free(options->source_map_file);
+    free(options->source_map_root);
     // Free custom functions
     free(options->c_functions);
     // Free custom importers
     free(options->c_importers);
     free(options->c_headers);
     // Reset our pointers
+    options->input_path = 0;
+    options->output_path = 0;
+    options->plugin_path = 0;
+    options->include_path = 0;
+    options->source_map_file = 0;
+    options->source_map_root = 0;
     options->c_functions = 0;
     options->c_importers = 0;
     options->c_headers = 0;
@@ -552,12 +592,6 @@ extern "C" {
     if (ctx->error_text)        free(ctx->error_text);
     if (ctx->error_json)        free(ctx->error_json);
     if (ctx->error_file)        free(ctx->error_file);
-    if (ctx->input_path)        free(ctx->input_path);
-    if (ctx->output_path)       free(ctx->output_path);
-    if (ctx->plugin_path)       free(ctx->plugin_path);
-    if (ctx->include_path)      free(ctx->include_path);
-    if (ctx->source_map_file)   free(ctx->source_map_file);
-    if (ctx->source_map_root)   free(ctx->source_map_root);
     free_string_array(ctx->included_files);
     // play safe and reset properties
     ctx->output_string = 0;
@@ -566,11 +600,6 @@ extern "C" {
     ctx->error_text = 0;
     ctx->error_json = 0;
     ctx->error_file = 0;
-    ctx->input_path = 0;
-    ctx->output_path = 0;
-    ctx->include_path = 0;
-    ctx->source_map_file = 0;
-    ctx->source_map_root = 0;
     ctx->included_files = 0;
     // now clear the options
     sass_clear_options(ctx);
@@ -587,6 +616,11 @@ extern "C" {
     free(compiler);
   }
 
+  void ADDCALL sass_delete_options (struct Sass_Options* options)
+  {
+    sass_clear_options(options); free(options);
+  }
+
   // Deallocate all associated memory with file context
   void ADDCALL sass_delete_file_context (struct Sass_File_Context* ctx)
   {