From: Francis James Franklin (f.j.franklin@sheffield.ac.uk)
Date: Thu Oct 09 2003 - 06:56:20 EDT
I know you haven't finished yet (and I'm really curious to see whether
the class profiles well or not, so thanks for doing this) but I have
some questions/comments/concerns:
1.The section from @@ -600,29 +583,38 @ seems to be a HashMap equality
check - it may be worth writing a UT_UTF8Hash operator==() for this - I
may do that. But my question is really about the use of strcmp()
instead of the UT_UTF8String operator==() which ought to perform
better...?
2. I'm a little concerned about this part of the patch:
@@ -771,16 +763,24 @@
if(!m_pAttributes)
return;
- UT_StringPtrMap::UT_Cursor _hc1(m_pAttributes);
+ unsigned int ui = 0;
+
const XML_Char * pEntry;
- for ( pEntry = static_cast<const XML_Char*>(_hc1.first());
_hc1.is_valid(); pEntry = static_cast<const XML_Char*>(_hc1.next()) )
+ for ( ui = 0; ui < m_pAttributes->count(); ui++)
{
+ const UT_UTF8String* key1;
+ const UT_UTF8String* value1;
+
+ m_pAttributes->pair( ui, key1, value1);
+
+ pEntry = value1->utf8_str();
+
if (pEntry && !*pEntry)
{
UT_ASSERT(!m_bIsReadOnly);
FREEP(pEntry);
- m_pAttributes->remove(_hc1.key(),pEntry);
+ m_pAttributes->del(*key1);
}
}
}
a. The FREEP(pEntry) is criminal.
b. This use of a for(;;) loop means that you will skip an entry
whenever you delete one.
c. This is just stripping values which have zero length? It may be
worth adding an option to UT_UTF8Hash itself to strip zero-length
values automatically...?
3. cch = UT_XML_strlen(s1) should really be cch = key->byteLength() +
1, or something like that
On Wednesday, October 8, 2003, at 11:17 PM, Robert Wilhelm wrote:
> I have made some test replacing eplacing UT_StringPtrMap with
> UT_UTF8Hash in pp_TableAttrProp.cpp.
> It is not finished, I just converted m_pAttributes but not
> yet m_pProperties. Also it might leak memory. It needs
> some cleanup. You name it.
>
> But at least it compiles and I was able to load the RTF spec.
> As I won´t be able to hack during the next days,
> I have attached my current patch so it cannot get lost.
> ML archives are a such a nice backup medium :-)
Regards, Frank
9. Has the work, for you, a metaphysical dimension? Yes ( ) No ( )
10. What is it (twenty-five words or less)?
- Snow White, Donald
Barthelme
This archive was generated by hypermail 2.1.4 : Thu Oct 09 2003 - 07:10:55 EDT