If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Clean-up macros in nsAttrValue.h and nsAttrValue.cpp

NEW
Unassigned

Status

()

Core
DOM: Core & HTML
8 years ago
8 years ago

People

(Reporter: mounir, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

8 years ago
This is a follow-up from bug 551670, see bug 551670 comment 59.

Quoting Jonas about nsAttrValue.h and nsAttrValue.cpp:
"
>+  const EnumTable* table = sEnumTableArray->
>+    ElementAt(allEnumBits & NS_ATTRVALUE_ENUMTABLEINDEX_MASK);

Hmm.. This looks wierd. This argument passed to ElementAt will always be a
multiple of 16384 or 16 or something like that. I think.

This bit twiddling is too complex and too spread out. I take full
responsibility for this fact.

Would you mind centralizing and cleaning it up? Something like the following
defines:

#define NS_ATTRVALUE_ENUM_TABLEINDEX_BITS (32 - 16 - 1 -
NS_ATTRVALUE_INTEGERTYPE_BITS)
#define NS_ATTRVALUE_ENUM_TABLEINDEX_OFFSET NS_ATTRVALUE_INTEGERTYPE_BITS
#define NS_ATTRVALUE_ENUM_TABLEINDEX_MASK \
 (PR_BITMASK(NS_ATTRVALUE_ENUM_TABLEINDEX_BITS) <<
NS_ATTRVALUE_ENUM_TABLEINDEX_OFFSET)
#define NS_ATTRVALUE_ENUM_GET_TABLEINDEX(_bits) \
 (((_bits) & NS_ATTRVALUE_ENUM_TABLEINDEX_MASK) >>
NS_ATTRVALUE_ENUM_TABLEINDEX_OFFSET)

#define NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER_BITS 1
#define NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER_OFFSET \
 (NS_ATTRVALUE_ENUM_TABLEINDEX_OFFSET + NS_ATTRVALUE_ENUM_TABLEINDEX_BITS)
#define NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER_MASK \
 (PR_BITMASK(NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER_BITS) <<
NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER_OFFSET)
#define NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER(_bits) \
 ((_bits) & NS_ATTRVALUE_ENUM_VALUE_NEEDS_TO_UPPER_MASK)

#define NS_ATTRVALUE_ENUM_VALUE_BITS 16
#define NS_ATTRVALUE_ENUM_VALUE_OFFSET \
 (NS_ATTRVALUE_ENUM_TABLEINDEX_OFFSET + NS_ATTRVALUE_ENUM_TABLEINDEX_BITS)
#define NS_ATTRVALUE_ENUM_VALUE_MASK \
 (PR_BITMASK(NS_ATTRVALUE_ENUM_VALUE_BITS) << NS_ATTRVALUE_ENUM_VALUE_OFFSET)
#define NS_ATTRVALUE_ENUM_GET_VALUE(_bits) \
 (((_bits) & NS_ATTRVALUE_ENUM_VALUE_MASK) >> NS_ATTRVALUE_ENUM_VALUE_OFFSET)

And then use these defines throughout. Feel free to simplify these macros.
Might actually be more readable to calculate the values by hand and #define to
those values directly.
"
You should use functions (marked as inline, not that it should make a difference) in preference to macros, for what it's worth -- easier to debug in/through.
You need to log in before you can comment on or make changes to this bug.