Closed Bug 1262324 Opened 4 years ago Closed 4 years ago

Two minor intl/locale/ tweaks

Categories

(Core :: Internationalization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

Attachments

(2 files)

I have two minor intl/locale/ tweaks that I stumbled across while looking at the size of Firefox's binary.
Currently, every entry in unixcharset.properties starts with "locale.all", and
then when searching them we prepend "locale.all" to the key.

This patch removes the prefixes in both cases, and saves 4 KiB in .text in the
process.
Attachment #8738337 - Flags: review?(VYV03354)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
I'm open to other suggestions for the new type name and its field names.
Attachment #8738347 - Flags: review?(VYV03354)
Comment on attachment 8738337 [details] [diff] [review]
(part 1) - Remove "locale.all" prefix from Unix charsets

Nice cleanup.
Attachment #8738337 - Flags: review?(VYV03354) → review+
Comment on attachment 8738347 [details] [diff] [review]
(part 2) - Introduce nsEncodingProp

Review of attachment 8738347 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with following comments fixed.

::: dom/encoding/EncodingUtils.cpp
@@ +14,5 @@
>  
>  namespace mozilla {
>  namespace dom {
>  
> +static const nsEncodingProp labelsEncodings[] = {

Maybe nsUConvProp is better. Obviously a langGroup is not an encoding.

::: intl/locale/nsUConvPropertySearch.cpp
@@ +33,5 @@
>    size_t index;
>    if (BinarySearchIf(aProperties, 0, aNumberOfProperties,
>                       PropertyComparator(flat), &index)) {
> +    nsDependentCString val(aProperties[index].mValue,
> +                           NS_PTR_TO_UINT32(aProperties[index].mValueLength));

NS_PTR_TO_UINT32 was a trick to store an integer to an element of a pointer array. Now this is unnecessary.

::: intl/locale/nsUConvPropertySearch.h
@@ +10,5 @@
> +struct nsEncodingProp
> +{
> +    const char* const mKey;
> +    const char* const mValue;
> +    const int32_t mValueLength;

uint32_t. length can't be negative.
Attachment #8738347 - Flags: review?(VYV03354) → review+
Thank you for the fast reviews.
https://hg.mozilla.org/mozilla-central/rev/978884d01e8e
https://hg.mozilla.org/mozilla-central/rev/835b1d9e4918
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Depends on: 1262799
You need to log in before you can comment on or make changes to this bug.