View Issue Details

IDProjectCategoryView StatusLast Update
0000475LDMud 3.3Runtimepublic2008-08-05 08:02
Reporterfufu Assigned Tofufu  
Status resolvedResolutionnot fixable 
Product Version3.3.712 
Summary0000475: adding m_allocate-d mappings with += is different from adding with +
DescriptionConsider this code:

  mixed x = m_allocate(100,2); x += ([2:3;4;5]);
  mixed y = m_allocate(100,2); y = y + ([2:3;4;5]);

The first line results in an error, saying the mapping width doesn't match. The second line results in y containing mapping of width 3. As x is empty, the first line should work as well.
Additional Informationadd_to_mapping() in mapping.c tests for empty mappings with
    0 == m2->num_entries && NULL == m2->hash
For m_allocate-d mappings, hash is empty but not 0.

Why is the hash tested at all? 3.2 doesn't have that check.

Attachment: patch to remove the NULL == m[1,2]->hash tests
TagsNo tags attached.


related to 0000291 closedfufu LDMud Implement a generic empty mapping 


2006-06-25 15:33


empty-mapping-check-fix.patch (716 bytes)   
Index: mapping.c
--- mapping.c	(revision 2306)
+++ mapping.c	(working copy)
@@ -3352,7 +3352,7 @@
         /* If one of the two mappings is empty, we can adjust its width
          * after getting rid of all pending data blocks.
-        if (0 == m2->num_entries && NULL == m2->hash)
+        if (0 == m2->num_entries)
             if (m2->cond != NULL)
@@ -3363,7 +3363,7 @@
             m2->num_values = m1->num_values;
-        else if (0 == m1->num_entries && NULL == m1->hash)
+        else if (0 == m1->num_entries)
             if (m1->cond != NULL)


2006-06-26 03:54

updater   ~0000508

While I agree that both code examples should be treated alike I would prefer in either case an error to be thrown:
if one really decides to use prior knowledge to avoid malloc overhead he should stick to the pre-set dimensions. Using m_allocate() instead of just initializing an empty mapping ([]) gives additional information which should be evaluated by the driver to avoid inconsistent use of variables.


2006-06-26 04:27

manager   ~0000509

I think what you want is covered by issue 291. I agree in principle, but this would be a incompatible change and requires more work than fixing this bug.


2006-07-18 13:07

reporter   ~0000513

This "mixed" behaviour also applies for 3.2.12 release, where I'd like to have it fixed, too.
I agree to Sorcerer: In case the first mapping was created either with m_allocate() or with ([:x]), the knowledge about the width of the mapping should be used consequently in any checks about different mapping widths. I doubt that it would break code, esp. as persons who pre-set a mapping dimension [should] _know_ what dimension they need later.


2008-08-04 23:05

manager   ~0000753

I still stand by my previous analysis -- the ->hash check is meant to identify mappings allocated by m_alloc(), but it's unreliable:

- it does not catch mappings created with m_allocate(0, width), or ([:width]).
- if the mapping gets compacted, the hashed part will go away.

The proper fix for this is to implement 0000291.

In the meantime, I'd still like to remove those ->hash checks.


2008-08-05 03:29

manager   ~0000754

I agree.


2008-08-05 07:58

manager   ~0000755

On second thought, the checks are necessary. If the mapping is a protector mapping and it has a hashed part, then the hashed part might be non-empty, even though the mapping itself is empty. In that case simply changing the num_values field of the mapping will cause trouble later, when the mapping's 'deleted' entries are freed.

So I'll leave the code as is and close this bug.

Issue History

Date Modified Username Field Change
2006-06-25 15:33 fufu New Issue
2006-06-25 15:33 fufu File Added: empty-mapping-check-fix.patch
2006-06-26 03:54 Sorcerer Note Added: 0000508
2006-06-26 04:27 fufu Note Added: 0000509
2006-07-18 13:07 Coogan Note Added: 0000513
2008-07-02 04:30 fufu Status new => assigned
2008-07-02 04:30 fufu Assigned To => fufu
2008-08-04 22:59 fufu Relationship added related to 0000291
2008-08-04 23:05 fufu Note Added: 0000753
2008-08-05 03:29 Gnomi Note Added: 0000754
2008-08-05 07:58 fufu Note Added: 0000755
2008-08-05 08:02 fufu Status assigned => resolved
2008-08-05 08:02 fufu Resolution open => not fixable