637607 - nsBaseHashtable::Get returns NULL, triggering build warning: converting to non-pointer type ‘unsigned int’ from NULL"

Status: NEW

(Core :: XPCOM, defect)

Description

[Daniel Holbert [:dholbert]]

Filing bug on this build warning in g++ 4.5:
=========
nsHTMLFormElement.cpp
In file included from ../../../../dist/include/nsDataHashtable.h:42:0,
                 from ../../../../dist/include/nsContentUtils.h:62,
                 from ../../../../../mozilla/content/html/content/src/../../../b
ase/src/nsGenericElement.h:58,
                 from ../../../../../mozilla/content/html/content/src/../../../b
ase/src/nsStyledElement.h:50,
                 from ../../../../../mozilla/content/html/content/src/../../../b
ase/src/nsMappedAttributeElement.h:48,
                 from ../../../../../mozilla/content/html/content/src/nsGenericH
TMLElement.h:41,
                 from ../../../../../mozilla/content/html/content/src/nsHTMLForm
Element.h:45,
                 from ../../../../../mozilla/content/html/content/src/nsHTMLForm
Element.cpp:37:
../../../../dist/include/nsBaseHashtable.h: In member function ‘UserDataType nsBaseHashtable<KeyClass, DataType, UserDataType>::Get(nsBaseHashtable<KeyClass, DataType, UserDataType>::KeyType) const [with KeyClass = nsStringCaseInsensitiveHashKey, DataType = unsigned int, UserDataType = unsigned int, nsBaseHashtable<KeyClass, DataType, UserDataType>::KeyType = const nsAString_internal&]’:
../../../../../mozilla/content/html/content/src/nsHTMLFormElement.cpp:2135:72:   instantiated from here
../../../../dist/include/nsBaseHashtable.h:150:14: warning: converting to non-pointer type ‘unsigned int’ from NULL
=========

The code in question is:

> 146   UserDataType Get(KeyType aKey) const
> 147   {
> 148     EntryType* ent = this->GetEntry(aKey);
> 149     if (!ent)
> 150       return NULL;

And the documentation higher up in the file says that it's perfectly fine for UserDataType to be a non-pointer type like an integer:
> 79  * @param UserDataType the user sees, for example PRUint32 or nsISupports*

Source file: http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsBaseHashtable.h

I think we should be able to just add an explicit "(UserDataType)" cast to the NULL value at nsBaseHashtable.h:150.

Comment 1

[Daniel Holbert [:dholbert]]

Just to be clear -- in this particular case, UserDataType is indeed an integer -- the hash table in question is...
> 422   nsDataHashtable<nsStringCaseInsensitiveHashKey,PRUint32> mRequiredRadioButtonCounts;
http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLFormElement.h#422

...and its "Get" method triggers this warning in the code below:
> 2134     mRequiredRadioButtonCounts.Put(aName,
> 2135                                    mRequiredRadioButtonCounts.Get(aName)+1);
http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLFormElement.cpp#2134

Comment 2

[Daniel Holbert [:dholbert]]

(er s/an integer/a PRUint32/ -- the point is, UserDataType is a non-pointer in this case)

Comment 3

[Benjamin Smedberg]

This form of "Get" was only ever intended for use with pointers (where values would typically never be null, and so null is a safe sentinel value). I really don't think I want to cast over this warning.

In this case, I think we should have an alternate form of Get which has an explicit default value, such as:

UserDataType Get(KeyType aKey, UserDataType default) const;