eliminate the only internal uses of HvFILL

The usages are as far as I know incorrect anyway. We resize
the hash bucket array based on the number of keys it holds,
not based on the number of buckets that are used, so this
usage was wrong anyway.

Another bug that this revealed is that the old code would allow
HvMAX(hv) to fall to 0, even though every other part of the
core expects it to have a minimum of 7 (meaning 8 buckets).

As part of this we change the hard coded 7 to a defined constant
PERL_HASH_DEFAULT_HvMAX.

After this patch there remains one use of HvFILL in core, that used
for scalar(%hash) which I plan to remove in a later patch.

diff --git a/hv.c b/hv.c
index 7d69fe4..ec1bfe8 100644 (file)
--- a/hv.c
+++ b/hv.c
@@ -1236,6 +1236,21 @@ Perl_hv_ksplit(pTHX_ HV *hv, IV newmax)
     }
 }
 
+/* IMO this should also handle cases where hv_max is smaller than hv_keys
+ * as tied hashes could play silly buggers and mess us around. We will
+ * do the right thing during hv_store() afterwards, but still - Yves */
+#define HV_SET_MAX_ADJUSTED_FOR_KEYS(hv,hv_max,hv_keys) STMT_START {\
+    /* Can we use fewer buckets? (hv_max is always 2^n-1) */        \
+    if (hv_max < PERL_HASH_DEFAULT_HvMAX) {                         \
+        hv_max = PERL_HASH_DEFAULT_HvMAX;                           \
+    } else {                                                        \
+        while (hv_max > PERL_HASH_DEFAULT_HvMAX && hv_max + 1 >= hv_keys * 2) \
+            hv_max = hv_max / 2;                                    \
+    }                                                               \
+    HvMAX(hv) = hv_max;                                             \
+} STMT_END
+
+
 HV *
 Perl_newHVhv(pTHX_ HV *ohv)
 {
@@ -1297,12 +1312,9 @@ Perl_newHVhv(pTHX_ HV *ohv)
        HE *entry;
        const I32 riter = HvRITER_get(ohv);
        HE * const eiter = HvEITER_get(ohv);
-       STRLEN hv_fill = HvFILL(ohv);
+        STRLEN hv_keys = HvTOTALKEYS(ohv);
 
-       /* Can we use fewer buckets? (hv_max is always 2^n-1) */
-       while (hv_max && hv_max + 1 >= hv_fill * 2)
-           hv_max = hv_max / 2;
-       HvMAX(hv) = hv_max;
+        HV_SET_MAX_ADJUSTED_FOR_KEYS(hv,hv_max,hv_keys);
 
        hv_iterinit(ohv);
        while ((entry = hv_iternext_flags(ohv, 0))) {
@@ -1341,7 +1353,7 @@ Perl_hv_copy_hints_hv(pTHX_ HV *const ohv)
 
     if (ohv) {
        STRLEN hv_max = HvMAX(ohv);
-       STRLEN hv_fill = HvFILL(ohv);
+        STRLEN hv_keys = HvTOTALKEYS(ohv);
        HE *entry;
        const I32 riter = HvRITER_get(ohv);
        HE * const eiter = HvEITER_get(ohv);
@@ -1349,9 +1361,7 @@ Perl_hv_copy_hints_hv(pTHX_ HV *const ohv)
        ENTER;
        SAVEFREESV(hv);
 
-       while (hv_max && hv_max + 1 >= hv_fill * 2)
-           hv_max = hv_max / 2;
-       HvMAX(hv) = hv_max;
+        HV_SET_MAX_ADJUSTED_FOR_KEYS(hv,hv_max,hv_keys);
 
        hv_iterinit(ohv);
        while ((entry = hv_iternext_flags(ohv, 0))) {
@@ -1377,6 +1387,7 @@ Perl_hv_copy_hints_hv(pTHX_ HV *const ohv)
     hv_magic(hv, NULL, PERL_MAGIC_hints);
     return hv;
 }
+#undef HV_SET_MAX_ADJUSTED_FOR_KEYS
 
 /* like hv_free_ent, but returns the SV rather than freeing it */
 STATIC SV*
@@ -1760,7 +1771,7 @@ Perl_hv_undef_flags(pTHX_ HV *hv, U32 flags)
     }
     if (!SvOOK(hv)) {
        Safefree(HvARRAY(hv));
-       xhv->xhv_max   = 7;     /* HvMAX(hv) = 7 (it's a normal hash) */
+        xhv->xhv_max = PERL_HASH_DEFAULT_HvMAX;        /* HvMAX(hv) = 7 (it's a normal hash) */
        HvARRAY(hv) = 0;
     }
     /* if we're freeing the HV, the SvMAGIC field has been reused for

diff --git a/hv.h b/hv.h
index 61bc5bd..270ae00 100644 (file)
--- a/hv.h
+++ b/hv.h
@@ -208,6 +208,8 @@ C<SV*>.
 =cut
 */
 
+#define PERL_HASH_DEFAULT_HvMAX 7
+
 /* these hash entry flags ride on hent_klen (for use only in magic/tied HVs) */
 #define HEf_SVKEY      -2      /* hent_key is an SV* */