Merge pull request #2335 from mgreter/code-hardening

mgreter · web-flow · commit f6a100efd8d1 · 2017-02-18T06:06:15.000+01:00
Code hardening
diff --git a/src/ast.cpp b/src/ast.cpp
@@ -961,13 +961,13 @@ namespace Sass {
 
     // do some magic we inherit from node and extend
     Node node = subweave(lhsNode, rhsNode);
-    Selector_List_Ptr result = SASS_MEMORY_NEW(Selector_List, pstate());
+    Selector_List_Obj result = SASS_MEMORY_NEW(Selector_List, pstate());
     NodeDequePtr col = node.collection(); // move from collection to list
     for (NodeDeque::iterator it = col->begin(), end = col->end(); it != end; it++)
     { result->append(nodeToComplexSelector(Node::naiveTrim(*it))); }
 
     // only return if list has some entries
-    return result->length() ? result : 0;
+    return result->length() ? result.detach() : 0;
 
   }
 
@@ -1688,31 +1688,31 @@ namespace Sass {
   void Arguments::adjust_after_pushing(Argument_Obj a)
   {
     if (!a->name().empty()) {
-      if (/* has_rest_argument_ || */ has_keyword_argument_) {
+      if (has_keyword_argument()) {
         error("named arguments must precede variable-length argument", a->pstate());
       }
-      has_named_arguments_ = true;
+      has_named_arguments(true);
     }
     else if (a->is_rest_argument()) {
-      if (has_rest_argument_) {
+      if (has_rest_argument()) {
         error("functions and mixins may only be called with one variable-length argument", a->pstate());
       }
       if (has_keyword_argument_) {
         error("only keyword arguments may follow variable arguments", a->pstate());
       }
-      has_rest_argument_ = true;
+      has_rest_argument(true);
     }
     else if (a->is_keyword_argument()) {
-      if (has_keyword_argument_) {
+      if (has_keyword_argument()) {
         error("functions and mixins may only be called with one keyword argument", a->pstate());
       }
-      has_keyword_argument_ = true;
+      has_keyword_argument(true);
     }
     else {
-      if (has_rest_argument_) {
+      if (has_rest_argument()) {
         error("ordinal arguments must precede variable-length arguments", a->pstate());
       }
-      if (has_named_arguments_) {
+      if (has_named_arguments()) {
         error("ordinal arguments must precede named arguments", a->pstate());
       }
     }
@@ -1867,7 +1867,7 @@ namespace Sass {
     // maybe convert to other unit
     // easier implemented on its own
     try { convert(prefered, strict); }
-    catch (incompatibleUnits& err)
+    catch (Exception::IncompatibleUnits& err)
     { error(err.what(), pstate()); }
     catch (...) { throw; }
 
diff --git a/src/ast.hpp b/src/ast.hpp
@@ -358,8 +358,11 @@ namespace Sass {
     void reset_duplicate_key() { duplicate_key_ = 0; }
     virtual void adjust_after_pushing(std::pair<Expression_Obj, Expression_Obj> p) { }
   public:
-    Hashed(size_t s = 0) : elements_(ExpressionMap(s)), list_(std::vector<Expression_Obj>())
-    { elements_.reserve(s); list_.reserve(s); reset_duplicate_key(); }
+    Hashed(size_t s = 0)
+    : elements_(ExpressionMap(s)),
+      list_(std::vector<Expression_Obj>()),
+      hash_(0), duplicate_key_(NULL)
+    { elements_.reserve(s); list_.reserve(s); }
     virtual ~Hashed();
     size_t length() const                  { return list_.size(); }
     bool empty() const                     { return list_.empty(); }
@@ -1036,9 +1039,13 @@ namespace Sass {
   class Content : public Statement {
     ADD_PROPERTY(Media_Block_Ptr, media_block)
   public:
-    Content(ParserState pstate) : Statement(pstate)
+    Content(ParserState pstate)
+    : Statement(pstate),
+      media_block_(NULL)
     { statement_type(CONTENT); }
-    Content(const Content* ptr) : Statement(ptr)
+    Content(const Content* ptr)
+    : Statement(ptr),
+      media_block_(ptr->media_block_)
     { statement_type(CONTENT); }
     ATTACH_AST_OPERATIONS(Content)
     ATTACH_OPERATIONS()
@@ -2217,22 +2224,22 @@ namespace Sass {
     void adjust_after_pushing(Parameter_Obj p)
     {
       if (p->default_value()) {
-        if (has_rest_parameter_) {
+        if (has_rest_parameter()) {
           error("optional parameters may not be combined with variable-length parameters", p->pstate());
         }
-        has_optional_parameters_ = true;
+        has_optional_parameters(true);
       }
       else if (p->is_rest_parameter()) {
-        if (has_rest_parameter_) {
+        if (has_rest_parameter()) {
           error("functions and mixins cannot have more than one variable-length parameter", p->pstate());
         }
-        has_rest_parameter_ = true;
+        has_rest_parameter(true);
       }
       else {
-        if (has_rest_parameter_) {
+        if (has_rest_parameter()) {
           error("required parameters must precede variable-length parameters", p->pstate());
         }
-        if (has_optional_parameters_) {
+        if (has_optional_parameters()) {
           error("required parameters must precede optional parameters", p->pstate());
         }
       }
@@ -2326,13 +2333,15 @@ namespace Sass {
     : AST_Node(pstate),
       contents_(c),
       connect_parent_(true),
-      media_block_(NULL)
+      media_block_(NULL),
+      hash_(0)
     { }
     Selector_Schema(const Selector_Schema* ptr)
     : AST_Node(ptr),
       contents_(ptr->contents_),
       connect_parent_(ptr->connect_parent_),
-      media_block_(ptr->media_block_)
+      media_block_(ptr->media_block_),
+      hash_(ptr->hash_)
     { }
     virtual bool has_parent_ref() const;
     virtual bool has_real_parent_ref() const;
diff --git a/src/b64/encode.h b/src/b64/encode.h
@@ -24,7 +24,9 @@ namespace base64
 
 		encoder(int buffersize_in = BUFFERSIZE)
 		: _buffersize(buffersize_in)
-		{}
+		{
+			base64_init_encodestate(&_state);
+		}
 
 		int encode(char value_in)
 		{
diff --git a/src/backtrace.hpp b/src/backtrace.hpp
@@ -60,13 +60,13 @@ namespace Sass {
 
     size_t depth()
     {
-      size_t d = 0;
+      size_t d = std::string::npos;
       Backtrace* p = parent;
       while (p) {
         ++d;
         p = p->parent;
       }
-      return d-1;
+      return d;
     }
 
   };
diff --git a/src/context.cpp b/src/context.cpp
@@ -72,6 +72,8 @@ namespace Sass {
     sheets(),
     subset_map(),
     import_stack(),
+    callee_stack(),
+    c_compiler(NULL),
 
     c_headers               (std::vector<Sass_Importer_Entry>()),
     c_importers             (std::vector<Sass_Importer_Entry>()),
diff --git a/src/cssize.cpp b/src/cssize.cpp
@@ -476,8 +476,6 @@ namespace Sass {
         ss->tabs(ss->tabs() + node->tabs());
         ss->group_end(node->group_end());
 
-        if (!ss) continue;
-
         Block_Obj bb = SASS_MEMORY_NEW(Block,
                                     children->pstate(),
                                     children->length(),
@@ -584,10 +582,11 @@ namespace Sass {
     }
 
     Media_Query_Ptr mm = SASS_MEMORY_NEW(Media_Query,
-
-mq1->pstate(), 0,
-mq1->length() + mq2->length(), mod == "not", mod == "only"
-);
+                                         mq1->pstate(),
+                                         0,
+                                         mq1->length() + mq2->length(),
+                                         mod == "not",
+                                         mod == "only");
 
     if (!type.empty()) {
       mm->media_type(SASS_MEMORY_NEW(String_Quoted, mq1->pstate(), type));
diff --git a/src/error_handling.cpp b/src/error_handling.cpp
@@ -111,7 +111,6 @@ namespace Sass {
     }
 
     IncompatibleUnits::IncompatibleUnits(const Number& lhs, const Number& rhs)
-    : lhs(lhs), rhs(rhs)
     {
       msg  = "Incompatible units: '";
       msg += rhs.unit();
@@ -120,6 +119,15 @@ namespace Sass {
       msg += "'.";
     }
 
+    IncompatibleUnits::IncompatibleUnits(const UnitType lhs, const UnitType rhs)
+    {
+      msg  = "Incompatible units: '";
+      msg += unit_to_string(rhs);
+      msg += "' and '";
+      msg += unit_to_string(lhs);
+      msg += "'.";
+    }
+
     AlphaChannelsNotEqual::AlphaChannelsNotEqual(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, const std::string& op)
     : lhs(lhs), rhs(rhs), op(op)
     {
diff --git a/src/error_handling.hpp b/src/error_handling.hpp
@@ -5,6 +5,8 @@
 #include <sstream>
 #include <stdexcept>
 #include "position.hpp"
+#include "ast_fwd_decl.hpp"
+#include "sass/functions.h"
 
 namespace Sass {
 
@@ -136,10 +138,11 @@ namespace Sass {
 
     class IncompatibleUnits : public OperationError {
       protected:
-        const Number& lhs;
-        const Number& rhs;
+        // const Sass::UnitType lhs;
+        // const Sass::UnitType rhs;
       public:
         IncompatibleUnits(const Number& lhs, const Number& rhs);
+        IncompatibleUnits(const UnitType lhs, const UnitType rhs);
         virtual ~IncompatibleUnits() throw() {};
     };
 
diff --git a/src/eval.cpp b/src/eval.cpp
@@ -1597,20 +1597,19 @@ namespace Sass {
 
     if (ltype == Expression::NULL_VAL) throw Exception::InvalidNullOperation(&lhs, &rhs, sass_op_to_name(op));
     if (rtype == Expression::NULL_VAL) throw Exception::InvalidNullOperation(&lhs, &rhs, sass_op_to_name(op));
-    if (op == Sass_OP::MOD) throw Exception::UndefinedOperation(&lhs, &rhs, sass_op_to_name(op));
-    if (op == Sass_OP::MUL) throw Exception::UndefinedOperation(&lhs, &rhs, sass_op_to_name(op));
     std::string sep;
     switch (op) {
       case Sass_OP::SUB: sep = "-"; break;
       case Sass_OP::DIV: sep = "/"; break;
-      case Sass_OP::MUL: sep = "*"; break;
-      case Sass_OP::MOD: sep = "%"; break;
+      // cases are already handled above
       case Sass_OP::EQ:  sep = "=="; break;
       case Sass_OP::NEQ:  sep = "!="; break;
       case Sass_OP::LT:  sep = "<"; break;
       case Sass_OP::GT:  sep = ">"; break;
       case Sass_OP::LTE:  sep = "<="; break;
       case Sass_OP::GTE:  sep = ">="; break;
+      case Sass_OP::MUL: throw Exception::UndefinedOperation(&lhs, &rhs, sass_op_to_name(op));
+      case Sass_OP::MOD: throw Exception::UndefinedOperation(&lhs, &rhs, sass_op_to_name(op));
       default:                      break;
     }
 
diff --git a/src/extend.cpp b/src/extend.cpp
@@ -2074,8 +2074,8 @@ namespace Sass {
         if (it.first) sel = it.first->first();
         if (it.second) ext = it.second;
         if (ext && (ext->extended() || ext->is_optional())) continue;
-        std::string str_sel(sel->to_string({ NESTED, 5 }));
-        std::string str_ext(ext->to_string({ NESTED, 5 }));
+        std::string str_sel(sel ? sel->to_string({ NESTED, 5 }) : "NULL");
+        std::string str_ext(ext ? ext->to_string({ NESTED, 5 }) : "NULL");
         // debug_ast(sel, "sel: ");
         // debug_ast(ext, "ext: ");
         error("\"" + str_sel + "\" failed to @extend \"" + str_ext + "\".\n"
diff --git a/src/functions.cpp b/src/functions.cpp
@@ -778,7 +778,7 @@ namespace Sass {
         return hsla_impl(hsl_struct.h, hsl_struct.s, hsl_struct.l, alpha, ctx, pstate);
       }
       if (a) {
-        double ascale = (a ? ARGR("$alpha", Number, -100.0, 100.0)->value() : 0.0) / 100.0;
+        double ascale = (ARGR("$alpha", Number, -100.0, 100.0)->value()) / 100.0;
         return SASS_MEMORY_NEW(Color,
                                pstate,
                                color->r(),
@@ -826,7 +826,7 @@ namespace Sass {
         return hsla_impl(hsl_struct.h, hsl_struct.s, hsl_struct.l, alpha, ctx, pstate);
       }
       if (a) {
-        double alpha = a ? ARGR("$alpha", Number, 0, 1.0)->value() : color->a();
+        double alpha = ARGR("$alpha", Number, 0, 1.0)->value();
         return SASS_MEMORY_NEW(Color,
                                pstate,
                                color->r(),
diff --git a/src/inspect.cpp b/src/inspect.cpp
@@ -15,7 +15,7 @@
 
 namespace Sass {
 
-  Inspect::Inspect(Emitter emi)
+  Inspect::Inspect(const Emitter& emi)
   : Emitter(emi)
   { }
   Inspect::~Inspect() { }
diff --git a/src/inspect.hpp b/src/inspect.hpp
@@ -17,7 +17,7 @@ namespace Sass {
 
   public:
 
-    Inspect(Emitter emi);
+    Inspect(const Emitter& emi);
     virtual ~Inspect();
 
     // statements
diff --git a/src/lexer.hpp b/src/lexer.hpp
@@ -96,9 +96,9 @@ namespace Sass {
     // Regex equivalent: /(?:literal)/
     template <const char* str>
     const char* exactly(const char* src) {
-      if (str == 0) return 0;
+      if (str == NULL) return 0;
       const char* pre = str;
-      if (src == 0) return 0;
+      if (src == NULL) return 0;
       // there is a small chance that the search string
       // is longer than the rest of the string to look at
       while (*pre && *src == *pre) {
@@ -114,9 +114,9 @@ namespace Sass {
     // only define lower case alpha chars
     template <const char* str>
     const char* insensitive(const char* src) {
-      if (str == 0) return 0;
+      if (str == NULL) return 0;
       const char* pre = str;
-      if (src == 0) return 0;
+      if (src == NULL) return 0;
       // there is a small chance that the search string
       // is longer than the rest of the string to look at
       while (*pre && (*src == *pre || *src+32 == *pre)) {
diff --git a/src/memory/SharedPtr.cpp b/src/memory/SharedPtr.cpp
@@ -37,22 +37,20 @@ namespace Sass {
     , dbg(false)
     #endif
   {
-      refcounter = 0;
-      #ifdef DEBUG_SHARED_PTR
-        if (taint) all.push_back(this);
-      #endif
-    };
-
-    SharedObj::~SharedObj() {
-      #ifdef DEBUG_SHARED_PTR
-          if (dbg) std::cerr << "Destruct " << this << "\n";
-          if(!all.empty()) { // check needed for MSVC (no clue why?)
-            all.erase(std::remove(all.begin(), all.end(), this), all.end());
-          }
-      #endif
-    };
-
+    refcounter = 0;
+    #ifdef DEBUG_SHARED_PTR
+      if (taint) all.push_back(this);
+    #endif
+  };
 
+  SharedObj::~SharedObj() {
+    #ifdef DEBUG_SHARED_PTR
+      if (dbg) std::cerr << "Destruct " << this << "\n";
+      if(!all.empty()) { // check needed for MSVC (no clue why?)
+        all.erase(std::remove(all.begin(), all.end(), this), all.end());
+      }
+    #endif
+  };
 
   void SharedPtr::decRefCount() {
     if (node) {
diff --git a/src/memory/SharedPtr.hpp b/src/memory/SharedPtr.hpp
diff --git a/src/node.cpp b/src/node.cpp
diff --git a/src/node.hpp b/src/node.hpp
diff --git a/src/parser.cpp b/src/parser.cpp
diff --git a/src/source_map.cpp b/src/source_map.cpp
diff --git a/src/units.cpp b/src/units.cpp
diff --git a/src/units.hpp b/src/units.hpp

Original file line number	Diff line number	Diff line change
`@@ -961,13 +961,13 @@ namespace Sass {`
`961`	`961`
`962`	`962`	`// do some magic we inherit from node and extend`
`963`	`963`	`Node node = subweave(lhsNode, rhsNode);`
`964`		`- Selector_List_Ptr result = SASS_MEMORY_NEW(Selector_List, pstate());`
	`964`	`+ Selector_List_Obj result = SASS_MEMORY_NEW(Selector_List, pstate());`
`965`	`965`	`NodeDequePtr col = node.collection(); // move from collection to list`
`966`	`966`	`for (NodeDeque::iterator it = col->begin(), end = col->end(); it != end; it++)`
`967`	`967`	`{ result->append(nodeToComplexSelector(Node::naiveTrim(*it))); }`
`968`	`968`
`969`	`969`	`// only return if list has some entries`
`970`		`- return result->length() ? result : 0;`
	`970`	`+ return result->length() ? result.detach() : 0;`
`971`	`971`
`972`	`972`	`}`
`973`	`973`
`@@ -1688,31 +1688,31 @@ namespace Sass {`
`1688`	`1688`	`void Arguments::adjust_after_pushing(Argument_Obj a)`
`1689`	`1689`	`{`
`1690`	`1690`	`if (!a->name().empty()) {`
`1691`		`- if (/* has_rest_argument_ \|\| */ has_keyword_argument_) {`
	`1691`	`+ if (has_keyword_argument()) {`
`1692`	`1692`	`error("named arguments must precede variable-length argument", a->pstate());`
`1693`	`1693`	`}`
`1694`		`- has_named_arguments_ = true;`
	`1694`	`+ has_named_arguments(true);`
`1695`	`1695`	`}`
`1696`	`1696`	`else if (a->is_rest_argument()) {`
`1697`		`- if (has_rest_argument_) {`
	`1697`	`+ if (has_rest_argument()) {`
`1698`	`1698`	`error("functions and mixins may only be called with one variable-length argument", a->pstate());`
`1699`	`1699`	`}`
`1700`	`1700`	`if (has_keyword_argument_) {`
`1701`	`1701`	`error("only keyword arguments may follow variable arguments", a->pstate());`
`1702`	`1702`	`}`
`1703`		`- has_rest_argument_ = true;`
	`1703`	`+ has_rest_argument(true);`
`1704`	`1704`	`}`
`1705`	`1705`	`else if (a->is_keyword_argument()) {`
`1706`		`- if (has_keyword_argument_) {`
	`1706`	`+ if (has_keyword_argument()) {`
`1707`	`1707`	`error("functions and mixins may only be called with one keyword argument", a->pstate());`
`1708`	`1708`	`}`
`1709`		`- has_keyword_argument_ = true;`
	`1709`	`+ has_keyword_argument(true);`
`1710`	`1710`	`}`
`1711`	`1711`	`else {`
`1712`		`- if (has_rest_argument_) {`
	`1712`	`+ if (has_rest_argument()) {`
`1713`	`1713`	`error("ordinal arguments must precede variable-length arguments", a->pstate());`
`1714`	`1714`	`}`
`1715`		`- if (has_named_arguments_) {`
	`1715`	`+ if (has_named_arguments()) {`
`1716`	`1716`	`error("ordinal arguments must precede named arguments", a->pstate());`
`1717`	`1717`	`}`
`1718`	`1718`	`}`
`@@ -1867,7 +1867,7 @@ namespace Sass {`
`1867`	`1867`	`// maybe convert to other unit`
`1868`	`1868`	`// easier implemented on its own`
`1869`	`1869`	`try { convert(prefered, strict); }`
`1870`		`- catch (incompatibleUnits& err)`
	`1870`	`+ catch (Exception::IncompatibleUnits& err)`
`1871`	`1871`	`{ error(err.what(), pstate()); }`
`1872`	`1872`	`catch (...) { throw; }`
`1873`	`1873`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,9 @@ namespace base64`
`24`	`24`
`25`	`25`	`encoder(int buffersize_in = BUFFERSIZE)`
`26`	`26`	`: _buffersize(buffersize_in)`
`27`		`- {}`
	`27`	`+ {`
	`28`	`+ base64_init_encodestate(&_state);`
	`29`	`+ }`
`28`	`30`
`29`	`31`	`int encode(char value_in)`
`30`	`32`	`{`
Original file line number	Diff line number	Diff line change
`@@ -60,13 +60,13 @@ namespace Sass {`
`60`	`60`
`61`	`61`	`size_t depth()`
`62`	`62`	`{`
`63`		`- size_t d = 0;`
	`63`	`+ size_t d = std::string::npos;`
`64`	`64`	`Backtrace* p = parent;`
`65`	`65`	`while (p) {`
`66`	`66`	`++d;`
`67`	`67`	`p = p->parent;`
`68`	`68`	`}`
`69`		`- return d-1;`
	`69`	`+ return d;`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`};`
Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,6 @@ namespace Sass {`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`IncompatibleUnits::IncompatibleUnits(const Number& lhs, const Number& rhs)`
`114`		`- : lhs(lhs), rhs(rhs)`
`115`	`114`	`{`
`116`	`115`	`msg = "Incompatible units: '";`
`117`	`116`	`msg += rhs.unit();`
`@@ -120,6 +119,15 @@ namespace Sass {`
`120`	`119`	`msg += "'.";`
`121`	`120`	`}`
`122`	`121`
	`122`	`+ IncompatibleUnits::IncompatibleUnits(const UnitType lhs, const UnitType rhs)`
	`123`	`+ {`
	`124`	`+ msg = "Incompatible units: '";`
	`125`	`+ msg += unit_to_string(rhs);`
	`126`	`+ msg += "' and '";`
	`127`	`+ msg += unit_to_string(lhs);`
	`128`	`+ msg += "'.";`
	`129`	`+ }`
	`130`	`+`
`123`	`131`	`AlphaChannelsNotEqual::AlphaChannelsNotEqual(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, const std::string& op)`
`124`	`132`	`: lhs(lhs), rhs(rhs), op(op)`
`125`	`133`	`{`