View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000475 | LDMud 3.3 | Runtime | public | 2006-06-25 15:33 | 2008-08-05 08:02 |
| Reporter | fufu | Assigned To | fufu | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | not fixable | ||
| Product Version | 3.3.712 | ||||
| Summary | 0000475: adding m_allocate-d mappings with += is different from adding with + | ||||
| Description | Consider 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 Information | add_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 | ||||
| Tags | No tags attached. | ||||
|
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)
{
|
|
|
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. |
|
|
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. |
|
|
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. |
|
|
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. |
|
|
I agree. |
|
|
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. |
| 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 |
