Status

()

enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sunfish, Assigned: sunfish)

Tracking

unspecified
mozilla34
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

Assignee

Description

5 years ago
Attached is a series of miscellaneous code-cleanup patches.
Attachment #8473722 - Flags: review?(nicolas.b.pierron)
Assignee

Comment 1

5 years ago
Attachment #8473723 - Flags: review?(nicolas.b.pierron)
Assignee

Comment 2

5 years ago
Attachment #8473724 - Flags: review?(nicolas.b.pierron)
Assignee

Comment 3

5 years ago
Posted patch static.patchSplinter Review
Attachment #8473725 - Flags: review?(nicolas.b.pierron)
Assignee

Comment 4

5 years ago
Posted patch const.patchSplinter Review
Attachment #8473726 - Flags: review?(nicolas.b.pierron)
Assignee

Comment 5

5 years ago
Attachment #8473727 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8473722 [details] [diff] [review]
unneeded-semicolons.patch

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

::: js/src/builtin/SIMD.cpp
@@ -689,5 @@
>      if (args.length() == 2) {
>          if (!IsVectorObject<V>(args[0]) || !args[1].isInt32())
>              return ErrorBadArgs(cx);
>  
> -        Elem *val = TypedObjectMemory<Elem *>(args[0]);;

Hum … I wonder where are the "case" and "esac".

::: js/src/jsapi.h
@@ +1325,1 @@
>  #endif

luke: Any reason to keep this code around inside a "#if 0" ?
Attachment #8473722 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8473723 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8473724 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8473725 - Flags: review?(nicolas.b.pierron) → review+
Attachment #8473726 - Flags: review?(nicolas.b.pierron) → review+
Comment on attachment 8473722 [details] [diff] [review]
unneeded-semicolons.patch

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

::: js/src/jsapi.h
@@ +1325,1 @@
>  #endif

luke> nbp: heh, no, it looks like some a mistake to leave that in
luke> nbp: actually, it seems better to use MOZ_STACK_CLASS
Comment on attachment 8473727 [details] [diff] [review]
const-gnostring.patch

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

::: js/xpconnect/src/XPCJSID.cpp
@@ +25,5 @@
>  
>  nsJSID::nsJSID()
> +    : mID(GetInvalidIID()),
> +      mNumber(const_cast<char *>(gNoString)),
> +      mName(const_cast<char *>(gNoString))

I am not a big fan of the const_cast, as it often hide mistakes.  But I did not spot any location where the content of mNumber / mName is mutated, and if ever it is, this will cause a SEGV as the gNoString would be mapped as read-only.

It might be better to change nsJSID to reflect that even if the pointer are mutable their target is not.
Attachment #8473727 - Flags: review?(nicolas.b.pierron) → review+
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.