Closed Bug 285751 Opened 20 years ago Closed 20 years ago

Move nsSchemaLoader from nsSupportsHashtable to nsInterfaceHashtable

Categories

(Core Graveyard :: Web Services, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: doronr, Assigned: doronr)

References

Details

Attachments

(1 file, 1 obsolete file)

 
Attachment #177141 - Flags: superreview?(peterv)
*** Bug 285625 has been marked as a duplicate of this bug. ***
Comment on attachment 177141 [details] [diff] [review]
patch, tested and seems to work fine.

>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================

> nsBuiltinSchemaCollection::nsBuiltinSchemaCollection()
> {
>+  mBuiltinTypesHash.Init();
>+  mSOAPTypeHash.Init();

These can fail. You need to add an Init method to nsBuiltinSchemaCollection
with these two calls and switch to NS_GENERIC_FACTORY_CONSTRUCTOR_INIT in the
module file.

> nsBuiltinSchemaCollection::~nsBuiltinSchemaCollection()
> {

>+  mBuiltinTypesHash.Clear();
>+  mSOAPTypeHash.Clear();

No need for this, nsInterfaceHashtable's destructor should clean up for you.

>     nsSchemaBuiltinType* builtin = new nsSchemaBuiltinType(typeVal);
>     if (!builtin) {
>       return NS_ERROR_OUT_OF_MEMORY;
>     }
>-    sup = builtin;
>-    mBuiltinTypesHash.Put(&key, sup);
>-    
>+
>+    mBuiltinTypesHash.Put(aName, builtin);
>+
>     *aType = builtin;
>     NS_ADDREF(*aType);

Make builtin an nsCOMPtr<nsISchemaType> and instead of addref'ing do
builtin.swap(*aType)


> nsSchemaLoader::nsSchemaLoader()
> {
>   mBuiltinCollection = do_GetService(NS_BUILTINSCHEMACOLLECTION_CONTRACTID);
>+  mSchemas.Init();

See above.

> }
> 
> nsSchemaLoader::~nsSchemaLoader()
> {
>+  mSchemas.Clear();

See above.

> nsSchemaLoader::GetSchema(const nsAString & targetNamespace, 
>                           nsISchema **_retval)
> {
>   NS_ENSURE_ARG_POINTER(_retval);
> 
>-  nsStringKey key(targetNamespace);
>-  nsCOMPtr<nsISupports> sup = dont_AddRef(mSchemas.Get(&key));
>-  nsCOMPtr<nsISchema> schema(do_QueryInterface(sup));
>-
>-  if (!schema) {
>+  if (!mSchemas.Get(targetNamespace, _retval)) {
>     return NS_ERROR_SCHEMA_UNKNOWN_TARGET_NAMESPACE;  // no schema for specified targetname
>   }
> 
>-  *_retval = schema;
>-  NS_ADDREF(*_retval);
>-
>   return NS_OK;

I'd just do

  return mSchemas.Get(targetNamespace, _retval) ? NS_OK :
NS_ERROR_SCHEMA_UNKNOWN_TARGET_NAMESPACE;

Wanna change _retval to aResult while you're here?

>+  nsISchema * os;
>+  if (mSchemas.Get(targetNamespace, &os)) {
>     *_retval = os;
>     NS_ADDREF(*_retval);
>     
>     return NS_OK;
>   }

This will leak, remove the NS_ADDREF.
Attachment #177141 - Flags: superreview?(peterv) → superreview-
Attached patch with commentsSplinter Review
Attachment #177141 - Attachment is obsolete: true
Attachment #177423 - Flags: superreview?(peterv)
Comment on attachment 177423 [details] [diff] [review]
with comments

>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================

>@@ -875,42 +852,42 @@ nsSchemaLoader::ProcessSchemaElement(nsI

>-  *_retval = schemaInst;
>-  NS_ADDREF(*_retval);
>+  *aResult = schemaInst;
>+  NS_ADDREF(*aResult);

Since you're touching this, make it:

  NS_ADDREF(*aResult = schemaInst);
Attachment #177423 - Flags: superreview?(peterv)
Attachment #177423 - Flags: superreview+
Attachment #177423 - Flags: review+
checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: