Remove implicit magic from OptVar copy operator and introduce oassign()

function instead.

src/compress_lzma.cpp

@@ -194,6 +194,7 @@ int upx_lzma_compress      ( const upx_bytep src, unsigned  src_len,
 
     int r = UPX_E_ERROR;
     HRESULT rh;
+    const lzma_compress_config_t *lcconf = cconf_parm ? &cconf_parm->conf_lzma : NULL;
     lzma_compress_result_t *res = &cresult->result_lzma;
 
     MyLzma::InStreamRam is; is.AddRef();
@@ -283,18 +284,13 @@ int upx_lzma_compress      ( const upx_bytep src, unsigned  src_len,
     }
 
     // cconf overrides
-    if (cconf_parm)
+    if (lcconf)
     {
-        if (cconf_parm->conf_lzma.pos_bits.is_set)
+        oassign(pr[0].uintVal, lcconf->pos_bits);
-            pr[0].uintVal = cconf_parm->conf_lzma.pos_bits;
+        oassign(pr[1].uintVal, lcconf->lit_pos_bits);
-        if (cconf_parm->conf_lzma.lit_pos_bits.is_set)
+        oassign(pr[2].uintVal, lcconf->lit_context_bits);
-            pr[1].uintVal = cconf_parm->conf_lzma.lit_pos_bits;
+        oassign(pr[3].uintVal, lcconf->dict_size);
-        if (cconf_parm->conf_lzma.lit_context_bits.is_set)
+        oassign(pr[5].uintVal, lcconf->num_fast_bytes);
-            pr[2].uintVal = cconf_parm->conf_lzma.lit_context_bits;
-        if (cconf_parm->conf_lzma.dict_size.is_set)
-            pr[3].uintVal = cconf_parm->conf_lzma.dict_size;
-        if (cconf_parm->conf_lzma.num_fast_bytes.is_set)
-            pr[5].uintVal = cconf_parm->conf_lzma.num_fast_bytes;
     }
 
     // limit dictionary size
@@ -302,12 +298,12 @@ int upx_lzma_compress      ( const upx_bytep src, unsigned  src_len,
         pr[3].uintVal = src_len;
 
     // limit num_probs
-    if (cconf_parm && cconf_parm->conf_lzma.max_num_probs)
+    if (lcconf && lcconf->max_num_probs)
     {
         for (;;)
         {
             unsigned n = 1846 + (768 << (pr[2].uintVal + pr[1].uintVal));
-            if (n <= cconf_parm->conf_lzma.max_num_probs)
+            if (n <= lcconf->max_num_probs)
                 break;
             if (pr[1].uintVal > pr[2].uintVal)
             {

src/conf.h

@@ -377,6 +377,14 @@ struct upx_callback_t
 
 #define ByteArray(var, size)    Array(unsigned char, var, size)
 
+struct nocopy
+{
+    inline nocopy() {}
+private:
+    nocopy(const nocopy &); // undefined
+    nocopy& operator=(const nocopy &); // undefined
+};
+
 
 /*************************************************************************
 // constants
@@ -475,19 +483,26 @@ struct OptVar
     static const T min_value_c = min_value;
     static const T max_value_c = max_value;
 
-    OptVar() : v(default_value), is_set(0) { }
+    void assertValue() {
-    OptVar& operator= (const OptVar& other) {
-        if (other.is_set) { v = other.v; is_set += 1; }
         // FIXME: this generates annoying warnings "unsigned >= 0 is always true"
         //assert((v >= min_value) && (v <= max_value));
-        return *this;
     }
+
+    OptVar() : v(default_value), is_set(0) { }
     OptVar& operator= (const T other) {
         v = other; is_set += 1;
-        // FIXME: this generates annoying warnings "unsigned >= 0 is always true"
+        assertValue();
-        //assert((v >= min_value) && (v <= max_value));
         return *this;
     }
+#if 0
+#error
+    // there is too much implicit magic in this copy operator;
+    // better introduce an explicit "oassign" function.
+    OptVar& operator= (const OptVar& other) {
+        if (other.is_set) { v = other.v; is_set += 1; }
+        assertValue(); return *this;
+    }
+#endif
 
     void reset() { v = default_value; is_set = 0; }
     operator T () const { return v; }
@@ -497,6 +512,21 @@ struct OptVar
 };
 
 
+// optional assignments
+template <class T> void oassign(T& self, const T& other) {
+    if (other.is_set) { self.v = other.v; self.is_set += 1; }
+}
+#if 0
+template <class V, class T> void oassign(V& v, const T& other) {
+    if (other.is_set) { v = other.v; }
+}
+#else
+template <class T> void oassign(unsigned& v, const T& other) {
+    if (other.is_set) { v = other.v; }
+}
+#endif
+
+
 struct lzma_compress_config_t
 {
     typedef OptVar<unsigned,  2u, 0u,   4u> pos_bits_t;             // pb

src/packer.cpp

@@ -191,12 +191,11 @@ bool Packer::compress(upx_bytep in, upx_bytep out,
     }
     if (M_IS_LZMA(ph.method))
     {
-        // NOTE: these are _optional_ assignments which query OptVar::is_set !
+        oassign(cconf.conf_lzma.pos_bits, opt->crp.crp_lzma.pos_bits);
-        cconf.conf_lzma.pos_bits = opt->crp.crp_lzma.pos_bits;
+        oassign(cconf.conf_lzma.lit_pos_bits, opt->crp.crp_lzma.lit_pos_bits);
-        cconf.conf_lzma.lit_pos_bits = opt->crp.crp_lzma.lit_pos_bits;
+        oassign(cconf.conf_lzma.lit_context_bits, opt->crp.crp_lzma.lit_context_bits);
-        cconf.conf_lzma.lit_context_bits = opt->crp.crp_lzma.lit_context_bits;
+        oassign(cconf.conf_lzma.dict_size, opt->crp.crp_lzma.dict_size);
-        cconf.conf_lzma.dict_size = opt->crp.crp_lzma.dict_size;
+        oassign(cconf.conf_lzma.num_fast_bytes, opt->crp.crp_lzma.num_fast_bytes);
-        cconf.conf_lzma.num_fast_bytes = opt->crp.crp_lzma.num_fast_bytes;
     }
     if (uip->ui_pass >= 0)
         uip->ui_pass++;

src/packmast.cpp

@@ -69,6 +69,7 @@ PackMaster::PackMaster(InputFile *f, struct options_t *o) :
     if (o)
     {
         this->local_options = *o;       // struct copy
+        //memcpy(&this->local_options, o, sizeof(*o)); // struct copy
         opt = &this->local_options;
     }
 }

src/util.h

@@ -124,14 +124,6 @@ int upx_tolower(int c);
 #endif
 
 
-class nocopy
-{
-    nocopy(const nocopy &); // undefined
-    nocopy& operator=(const nocopy &); // undefined
-public:
-    inline nocopy() {}
-};
-
 #endif /* already included */