784739 - Switch from NULL to nullptr

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(no longer active)]

This is the follow-up to bug 626472.

Comment 1

[(no longer active)]

Josh, do you know any contributors who might wanna take this over?  This is not necessarily a good first bug.

Comment 2

[Kévin Plestan]

What must we do for resolve this "bug" please ? Just replace all NULL by nullptr ?

Comment 3

[Kévin Plestan]

And, must we have Mac for this "bug" ?

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Kévin Plestan from comment #2)
> What must we do for resolve this "bug" please ? Just replace all NULL by
> nullptr ?

Yes.

(In reply to Kévin Plestan from comment #3)
> And, must we have Mac for this "bug" ?

No.

Comment 5

[Kévin Plestan]

Ok Thank you, I will try to do this

Comment 6

[Kévin Plestan]

Excuse me, must I search file by file "NULL" and replace it by "nullptr" or there is something which can help me ? Something like an IDE ? Thank you for your help.

Comment 7

[Kévin Plestan]

Must I replace all NULL I see by nullptr or must I just replace the NULL which are used for pointer ? Sorry for silly questions but I do not know all of the programming's world.

Comment 8

[(no longer active)]

Thanks Kévin for your interest in working on this!

I think the best way to replace NULL with nullptr across all of our code is to use a script similar to the one used in bug 690892 <https://bug690892.bugzilla.mozilla.org/attachment.cgi?id=567444>.  You can just replace the sed rules there with ones that replace NULL with nullptr, something like:

's/\bNULL    /nullptr /g'
's/\bNULL\b/nullptr/g'

The spacing trick is used to make sure that we avoid messing up the indentation as much as we can.

Please let me know if you need more help.

Comment 9

[Kévin Plestan]

Must I modified .idl files ?

Thank you.

Comment 10

[(no longer active)]

(In reply to comment #9)
> Must I modified .idl files ?

No.

Comment 11

[Kévin Plestan]

Thank you very much

Comment 12

[Shashank Gupta]

@Kevin: I'd love to take this over if you're no longer working on this. :)

Comment 13

[Kévin Plestan]

I'm at the end of my work. I'm trying to compile my mozilla's version. I'll informe you if it's working ;) .

P.S : correct me if I do english errors because I'm not english (but french. Besides, a french who's learning english ^^ ) Thank You !

Comment 14

[(no longer active)]

(In reply to comment #13)
> I'm at the end of my work. I'm trying to compile my mozilla's version. I'll
> informe you if it's working ;) .

Great to hear that!

> P.S : correct me if I do english errors because I'm not english (but french.
> Besides, a french who's learning english ^^ ) Thank You !

No worries.  A lot of us are not native English speakers either (including myself.  :-)

Comment 15

[Shashank Gupta]

@Kevin: Still working on this? I'd love to fix this bug if you're no longer interested. :)

Comment 16

[Cristian Ojog [:crisojog]]

I've tried changing NULL to nullptr in all cpp, cc, mm and h files and, because I changed source/mfbt/Assertions.h which is used by memory/mozjemalloc/jemalloc.c so this gives me an error. Should I avoid changing the h files?

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Cristian Ojog from comment #16)
> I've tried changing NULL to nullptr in all cpp, cc, mm and h files and,
> because I changed source/mfbt/Assertions.h which is used by
> memory/mozjemalloc/jemalloc.c so this gives me an error. Should I avoid
> changing the h files?

The majority of h files can use nullptr; I think avoiding Assertions.h will probably be sufficient.

Comment 18

[[:badanomaly]]

Hi I tried this bug with command 
find ./ ! -path ./.hg/\* ! -name Assertions.h -type f \( -iname \*.h -o -iname \*.cpp -o -iname *.mm -o -iname *.cc \) | xargs -n 1 sed -i -e 's/\bNULL /nullptr /g' -e 's/\bNULL\b/nullptr/g'

but the build is showing
error: ‘nullptr’ undeclared (first use in this function)
do i have to include <mfbt/nullptr.h> to files too ?
and which directories do i have to exclude other than .hg ?

Comment 19

[(no longer active)]

(In reply to comment #18)
> Hi I tried this bug with command 
> find ./ ! -path ./.hg/\* ! -name Assertions.h -type f \( -iname \*.h -o -iname
> \*.cpp -o -iname *.mm -o -iname *.cc \) | xargs -n 1 sed -i -e 's/\bNULL
> /nullptr /g' -e 's/\bNULL\b/nullptr/g'
> 
> but the build is showing
> error: ânullptrâ undeclared (first use in this function)
> do i have to include <mfbt/nullptr.h> to files too ?
> and which directories do i have to exclude other than .hg ?

Which file is this error coming from?

Comment 20

[[:badanomaly]]

/home/mohit/mozilla-central/memory/mozjemalloc/jemalloc.c: In function ‘extent_tree_szad_first’:
/home/mohit/mozilla-central/memory/mozjemalloc/jemalloc.c:2307:1: error: ‘nullptr’ undeclared (first use in this function)

Same error in functions
extent_tree_szad_[first/last/next/prev/nsearch/search/psearch]
extent_tree_ad_[first/last/next/prev/nsearch/search/psearch]
arena_chunk_tree_dirty_[first/last/next/prev/nsearch/search/psearch]
arena_avail_tree_[first/last/next/prev/nsearch/search/psearch]

Comment 21

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

You probably should leave memory/mozjemalloc and memory/jemalloc alone.

Comment 22

[[:badanomaly]]

(In reply to :Ms2ger from comment #21)
> You probably should leave memory/mozjemalloc and memory/jemalloc alone.

Now these error are present
/home/mohit/mozilla-central/nsprpub/pr/src/io/prfdcach.c: In function ‘_PR_Getfd’:
 /home/mohit/mozilla-central/nsprpub/pr/src/io/prfdcach.c:113:33: error: ‘nullptr’ undeclared (first use in this function)
 /home/mohit/mozilla-central/nsprpub/pr/src/io/prfdcach.c:113:33: note: each undeclared identifier is reported only once for each function it appears in
 /home/mohit/mozilla-central/nsprpub/pr/src/io/prfdcach.c: In function ‘_PR_CleanupFdCache’:
  /home/mohit/mozilla-central/nsprpub/pr/src/io/prfdcach.c:274:9: error: ‘nullptr’ undeclared (first use in this function)

I modified the command to :
find ./  ! -path ./.hg/\* ! -path ./memory/mozjemalloc/\* ! -path ./memory/jemalloc/\* ! -name Assertions.h -type f \( -iname \*.h -o -iname \*.cpp -o -iname *.mm -o -iname *.cc \) | xargs -n 1 sed -i -e 's/\bNULL /nullptr /g' -e 's/\bNULL\b/nullptr/g'

Comment 23

[(no longer active)]

Please exclude all .c files.  Thanks!

Comment 24

[[:badanomaly]]

Attachment: NULL changed to nullptr in .cc and .cpp files

Comment 25

[[:badanomaly]]

Attachment: command used to create the patch

Comment 26

[[:badanomaly]]

I excluded mozilla-central/toolkit/* because it was showing an "nullptr" not defined error in files
mozilla-central/toolkit/crashreporter/google-breakpad/src/common/*.cc

and the patch was too large so i had to upload the compressed version.

Comment 27

[[:badanomaly]]

Attachment: script

Comment 28

[[:badanomaly]]

Attachment: NULL changed to nullptr in .cc and .cpp files

Comment 29

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Thanks for the patch; I'll try to take a look tomorrow.

Comment 30

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 701300 [details] [diff] [review]
NULL changed to nullptr in .cc and .cpp files

Sorry it took a little longer than I expected. I skimmed over the patch, and it generally looks good. There's a few more things you should leave alone, though:


You shouldn't touch the strings in dom/indexedDB/OpenDatabaseHelper.cpp.

Please fix the indentation of the backslashes of COPY_STRING in dom/workers/Navigator.cpp and dom/workers/WorkerScope.cpp (they should all line up).

extensions/spellcheck/hunspell/src/ is upstream code; you should leave that alone.

Same for gfx/angle/, gfx/cairo/cairo/src/, gfx/graphite2/, gfx/harfbuzz/src/, gfx/ots/src/, gfx/skia/src/ and gfx/ycbcr/.

Same for media/libsoundtouch/, mfbt/double-conversion/ and other-licenses/.


Thanks for taking this on!

Comment 31

[[:badanomaly]]

Attachment: bug 784739 fix

Comment 32

[[:badanomaly]]

Attachment: command

Comment 33

[(no longer active)]

Comment on attachment 706564 [details]
command

>find ./ \
>! -path ./.hg/\* \
>! -path ./extensions/spellcheck/hunspell/src\* \
>! -path ./toolkit\* \

Why are you excluding toolkit?

>! -path \*netwerk/sctp\* \
>! -path \*xpcom/typelib/xpt/\* \
>! -path ./memory/mozjemalloc/\* \
>! -path ./js\* ! -path \*nsprpub\* \
>! -path ./memory/jemalloc/\* \
>! -path ./gfx/angle/\* \
>! -path ./gfx/cairo/cairo/src/\* \
>! -path ./gfx/graphite2/\* \
>! -path ./gfx/harfbuzz/src/\* \
>! -path ./gfx/ots/src/\* \
>! -path ./gfx/skia/src/\* \
>! -path ./gfx/ycbcr/\* \
>! -path ./media/libsoundtouch/\* \
>! -path ./mfbt/double-conversion/\* \
>! -path ./other-licenses/\* \
>! -name Assertions.h \
>! -name OpenDatabaseHelper.cpp \
>-type f \( -iname \*.cpp -o -iname *.cc \) | xargs -n 1 sed -i -e 's/\bNULL /nullptr /g' -e 's/\bNULL\b/nullptr/g'

This will mess up indentations.  You want something like: 's/\bNULL    /nullptr /g'.

Also, have you run this through the try server?  If yes, please post a TBPL link.  Thanks!

Comment 34

[(no longer active)]

Randell, what parts of the code in media/webrtc is owned by us?

Comment 35

[Randell Jesup [:jesup] (needinfo me)]

Well, these are not our code:

media/webrtc/trunk
media/webrtc/signaling, except for:
  media/webrtc/signaling/src/test
  media/webrtc/signaling/src/media-conduit
  media/webrtc/signaling/src/mediapipeline
  media/webrtc/signaling/src/peerconnection

The media/webrtc/signaling code other than ours has largely been taken over by us though, so we can discuss making reasonably portable changes to it for stuff like this.  Ethan Hugg of Cisco (ehugg on #media) and I would be the people to talk to.

In other news, media/mtransport/third_party are all imported, as is netwerk/srtp/src and netwerk/sctp/src

Comment 36

[(no longer active)]

Thanks, Randell!

So, Mohit, to keep things simper here, I'd suggest you exclude media/webrtc, media/mtransport/third_party, netwerk/srtp/src and netwerk/sctp/src for now.  Thanks!

Comment 37

[[:badanomaly]]

Attachment: NULL changed to nullptr

Comment 38

[[:badanomaly]]

Attachment: Updated command

Comment 39

[(no longer active)]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=ae390dc0b45d

Comment 40

[(no longer active)]

As you can see in the link in comment 39, this patch doesn't build on any platform.

Comment 41

[matekm]

Hi all, 

as there was no activity in this ticket since end of January, I would like to take it over. Is it ok to re-assign it to me?

Comment 42

[matekm]

Attachment: NULL to nullptr switch diff - version 1 (2012-03-21)

This is first version of the patch that changes occurences of NULL to nullptr

Comment 43

[matekm]

Would like to run this patch through try server, but I need level 1 access and for that I need a voucher. Please, vote on https://bugzilla.mozilla.org/show_bug.cgi?id=851871

Comment 44

[matekm]

Attachment: NULL to nullptr switch diff - version 2 (2012-03-21) - gzipped

New, gzipped patch. Previous one wasn't full.If there is a need I can split this patch in few smaller ones.

Comment 45

[Randell Jesup [:jesup] (needinfo me)]

Instead of gzipping it, I advise breaking it into one patch per top-level directory (or some such)

Comment 46

[matekm]

Attachment: mozilla-central/accessible subdir NULL -> nullptr switch

Comment 47

[matekm]

Attachment: mozilla-central/b2g subdir NULL -> nullptr switch

Comment 48

[matekm]

Attachment: mozilla-central/browser subdir NULL -> nullptr switch

Comment 49

[matekm]

Attachment: mozilla-central/build subdir NULL -> nullptr switch

Comment 50

[matekm]

Attachment: mozilla-central/caps subdir NULL -> nullptr switch

Comment 51

[matekm]

Attachment: mozilla-central/chrome subdir NULL -> nullptr switch

Comment 52

[matekm]

Attachment: mozilla-central/config subdir NULL -> nullptr switch

Comment 53

[matekm]

Attachment: mozilla-central/content subdir NULL -> nullptr switch

Comment 54

[matekm]

Attachment: mozilla-central/dbm subdir NULL -> nullptr switch

Comment 55

[matekm]

Attachment: mozilla-central/docshell subdir NULL -> nullptr switch

Comment 56

[matekm]

Attachment: mozilla-central/dom subdir NULL -> nullptr switch

Comment 57

[matekm]

Attachment: mozilla-central/editor subdir NULL -> nullptr switch

Comment 58

[matekm]

Attachment: mozilla-central/embedding subdir NULL -> nullptr switch

Comment 59

[matekm]

Attachment: mozilla-central/extensions subdir NULL -> nullptr switch

Comment 60

[matekm]

Attachment: mozilla-central/gfx subdir NULL -> nullptr switch

Comment 61

[Randell Jesup [:jesup] (needinfo me)]

Why are these all assigned for yourself to review?

Comment 62

[(no longer active)]

I don't expect to get to these reviews before next week...  Sorry.

Comment 63

[(no longer active)]

Comment on attachment 729649 [details] [diff] [review]
mozilla-central/editor subdir NULL -> nullptr switch

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

::: editor/libeditor/html/nsEditorTxnLog.cpp
@@ +307,4 @@
>    if (txn) {
>      txn->GetTxnDescription(str);
>      if (str.IsEmpty())
> +      str.AssignLiteral("<nullptr>");

Hmm, this is bad, but it doesn't matter in this case since this code is unused...

Comment 64

[(no longer active)]

Comment on attachment 729649 [details] [diff] [review]
mozilla-central/editor subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8abe42df0dc

Comment 65

[(no longer active)]

Comment on attachment 729643 [details] [diff] [review]
mozilla-central/config subdir NULL -> nullptr switch

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

This file is not used, and it should probably be removed.  Can you please file a bug on that?  Thanks!

Comment 66

[(no longer active)]

Comment on attachment 729642 [details] [diff] [review]
mozilla-central/chrome subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/275cff0aef99

Comment 67

[(no longer active)]

Comment on attachment 729641 [details] [diff] [review]
mozilla-central/caps subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/832de954da6a

Comment 68

[(no longer active)]

Comment on attachment 729635 [details] [diff] [review]
mozilla-central/b2g subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ab6aaf3ed13

Comment 69

[(no longer active)]

Comment on attachment 729645 [details] [diff] [review]
mozilla-central/dbm subdir NULL -> nullptr switch

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

We do not own this code.  Sorry, I should have mentioned it earlier.

Comment 70

[(no longer active)]

Comment on attachment 729650 [details] [diff] [review]
mozilla-central/embedding subdir NULL -> nullptr switch

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

::: embedding/components/find/src/nsFind.cpp
@@ +466,4 @@
>  {
>    if (!aNode)
>    {
> +    printf(">>>> Node: nullptr\n");

This is not needed, but I'll remove it myself.

Comment 71

[(no longer active)]

Comment on attachment 729646 [details] [diff] [review]
mozilla-central/docshell subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b2c0b85aea3

Comment 72

[(no longer active)]

Comment on attachment 729650 [details] [diff] [review]
mozilla-central/embedding subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/16af89e4b032

Comment 73

[(no longer active)]

Comment on attachment 729634 [details] [diff] [review]
mozilla-central/accessible subdir NULL -> nullptr switch

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/6120f41441ff

Comment 74

[(no longer active)]

Comment on attachment 729637 [details] [diff] [review]
mozilla-central/browser subdir NULL -> nullptr switch

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e56f91d780df

Comment 75

[(no longer active)]

Comment on attachment 729651 [details] [diff] [review]
mozilla-central/extensions subdir NULL -> nullptr switch

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/e0d063f04ee2

Comment 76

[(no longer active)]

Comment on attachment 729644 [details] [diff] [review]
mozilla-central/content subdir NULL -> nullptr switch

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

::: content/smil/nsSMILAnimationFunction.cpp
@@ +421,5 @@
>      }
>  
>      if (NS_SUCCEEDED(rv)) {
> +      NS_ABORT_IF_FALSE(from, "nullptr from-value during interpolation");
> +      NS_ABORT_IF_FALSE(to, "nullptr to-value during interpolation");

These are not needed, will fix myself.

::: content/smil/nsSMILNullType.cpp
@@ +33,4 @@
>  nsSMILNullType::Add(nsSMILValue& aDest, const nsSMILValue& aValueToAdd,
>                      uint32_t aCount) const
>  {
> +  NS_NOTREACHED("Adding nullptr type");

Ditto.

::: content/smil/nsSMILTimedElement.cpp
@@ +267,4 @@
>  void
>  nsSMILTimedElement::SetAnimationElement(SVGAnimationElement* aElement)
>  {
> +  NS_ABORT_IF_FALSE(aElement, "nullptr owner element");

Ditto.

::: content/svg/content/src/SVGContentUtils.cpp
@@ +70,4 @@
>  float
>  SVGContentUtils::GetFontSize(nsIFrame *aFrame)
>  {
> +  NS_ABORT_IF_FALSE(aFrame, "nullptr frame in GetFontSize");

And here.

::: content/svg/content/src/nsSVGAttrTearoffTable.h
@@ +55,4 @@
>  #endif
>      mTable.Get(aSimple, &tearoff);
>    NS_ABORT_IF_FALSE(!found || tearoff,
> +      "nullptr pointer stored in attribute tear-off map");

And here.

Comment 77

[(no longer active)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #74)
> Comment on attachment 729637 [details] [diff] [review]
> mozilla-central/browser subdir NULL -> nullptr switch
> 
> Review of attachment 729637 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/e56f91d780df

Landed a follow-up: https://hg.mozilla.org/integration/mozilla-inbound/rev/06ae36377cfa

Comment 78

[(no longer active)]

Comment on attachment 729637 [details] [diff] [review]
mozilla-central/browser subdir NULL -> nullptr switch

This broke the build again this time somewhere else on Windows, so I backed it out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/84588077a188

Build failure:

https://tbpl.mozilla.org/php/getParsedLog.php?id=21369991&tree=Mozilla-Inbound

Comment 79

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e8abe42df0dc
https://hg.mozilla.org/mozilla-central/rev/275cff0aef99
https://hg.mozilla.org/mozilla-central/rev/832de954da6a
https://hg.mozilla.org/mozilla-central/rev/2ab6aaf3ed13
https://hg.mozilla.org/mozilla-central/rev/5b2c0b85aea3
https://hg.mozilla.org/mozilla-central/rev/16af89e4b032
https://hg.mozilla.org/mozilla-central/rev/6120f41441ff
https://hg.mozilla.org/mozilla-central/rev/e0d063f04ee2

Comment 80

[(no longer active)]

Comment on attachment 729644 [details] [diff] [review]
mozilla-central/content subdir NULL -> nullptr switch

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad1a3999aa2e

Comment 81

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ad1a3999aa2e

Comment 82

[(no longer active)]

Comment on attachment 729648 [details] [diff] [review]
mozilla-central/dom subdir NULL -> nullptr switch

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

::: dom/plugins/base/android/android_npapi.h
@@ +104,4 @@
>  
>  /** queries for a specific ANPInterface.
>  
> +    Maybe called with nullptr for the NPP instance

Please revert the changes to this file.

::: dom/plugins/base/npapi.h
@@ +154,4 @@
>    void*    notifyData;
>    const    char* headers; /* Response headers from host.
>                             * Exists only for >= NPVERS_HAS_RESPONSE_HEADERS.
> +                           * Used for HTTP only; nullptr for non-HTTP.

And this file.

::: dom/plugins/base/npruntime.h
@@ +137,4 @@
>  #define VOID_TO_NPVARIANT(_v)                                                 \
>  NP_BEGIN_MACRO                                                                \
>      (_v).type = NPVariantType_Void;                                           \
> +    (_v).value.objectValue = nullptr;                                            \

And this file.

::: dom/system/gonk/android_audio/EffectApi.h
@@ +78,4 @@
>      uint8_t node[6];
>  } effect_uuid_t;
>  
> +// nullptr UUID definition (matches SL_IID_NULL_)

And this file.

Comment 83

[(no longer active)]

Comment on attachment 729653 [details] [diff] [review]
mozilla-central/gfx subdir NULL -> nullptr switch

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

Please revert the changes to gfx/skia, and separate out the gfx/2d changes into another patch.  Thanks!

Comment 84

[matekm]

Sorry for that little mess with changes. Next time I will push the patches to the try server...

Comment 85

[matekm]

Attachment: mozilla-central/browser subdir NULL -> nullptr switch

Try server: https://tbpl.mozilla.org/?tree=Try&rev=62200a7dbc17

Comment 86

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

For the dom/ changes, you'll need to revert your changes to dom/indexedDB/OpenDatabaseHelper.cpp.

Comment 87

[tvssarma.omega9]

Hi, this is probably a very noobish question, but what would happen if I made a #define somewhere and changed NULL to nullptr? To prevent mixing C++ and C we could do this in an appropriate header file or add a level of abstraction. I'd like to work on this way of doing it and close this bug.

Comment 88

[matekm]

Last patch don't include dom changes, only browser/

Comment 89

[(no longer active)]

(In reply to tvssarma.omega9 from comment #87)
> Hi, this is probably a very noobish question, but what would happen if I
> made a #define somewhere and changed NULL to nullptr? To prevent mixing C++
> and C we could do this in an appropriate header file or add a level of
> abstraction.

The point of this bug is to use nullptr in our source code instead of NULL for consistency.

> I'd like to work on this way of doing it and close this bug.

I believe matekm is already working on the rest of the changes needed here...

Comment 90

[(no longer active)]

Comment on attachment 734213 [details] [diff] [review]
mozilla-central/browser subdir NULL -> nullptr switch

This patch is weird..  It seems like you've concatenated a few patch files here?  Can you please upload a version of the patch which includes all of the changes?  If you have several mq patches in your queue, you can use hg diff -r to get the difference between two changesets.

Comment 91

[Josh Matthews [:jdm]]

Could you confirm that you're still working on this?

Comment 92

[Biru [:poiru]]

Assigning myself since matekm does not seem to be working on this any longer.

Comment 93

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in accessible/

Comment 94

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/2d/

Comment 95

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/gl/

Comment 96

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/ipc/

Comment 97

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/

Comment 98

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/composite/

Comment 99

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/d3d9/

Comment 100

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/d3d10/

Comment 101

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/ipc/

Comment 102

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/opengl/

Comment 103

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/src/

Comment 104

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/d3d11/

Comment 105

[Biru [:poiru]]

Ehsan: By the way, if it is easier for you to review a few large patches (compared to many smaller patches), please let me know and I'll do so in the future.

Comment 106

[(no longer active)]

Comment on attachment 775350 [details] [diff] [review]
Switch from NULL to nullptr in gfx/2d/

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

Redirecting request to Bas.

Comment 107

[(no longer active)]

Comment on attachment 775362 [details] [diff] [review]
Switch from NULL to nullptr in gfx/layers/opengl/

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

::: gfx/layers/opengl/LayerManagerOGL.h
@@ +15,5 @@
>  #ifdef XP_WIN
>  #include <windows.h>
>  #endif
>  
> +#define BUFFER_OFFSET(i) ((char *)nullptr + (i))

This code looks weird.  Joe, what's this all about?

Comment 108

[(no longer active)]

(In reply to comment #105)
> Ehsan: By the way, if it is easier for you to review a few large patches
> (compared to many smaller patches), please let me know and I'll do so in the
> future.

Smaller patches are always better!

Comment 109

[Joe Drew (not getting mail)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #107)
> > +#define BUFFER_OFFSET(i) ((char *)nullptr + (i))
> 
> This code looks weird.  Joe, what's this all about?

Its a pretty common idiom: http://www.embedded.com/design/embedded/4024941/Learn-a-new-trick-with-the-offsetof--macro

Comment 110

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/2d/

Rebased patch.

Comment 111

[(no longer active)]

(In reply to comment #109)
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #107)
> > > +#define BUFFER_OFFSET(i) ((char *)nullptr + (i))
> > 
> > This code looks weird.  Joe, what's this all about?
> 
> Its a pretty common idiom:
> http://www.embedded.com/design/embedded/4024941/Learn-a-new-trick-with-the-offsetof--macro

Gah, I read that as:

(char)nullptr + (i)

Sorry!

Comment 112

[(no longer active)]

Bas: ping?

Comment 113

[Bas Schouten (:bas.schouten)]

Comment on attachment 775809 [details] [diff] [review]
Switch from NULL to nullptr in gfx/2d/

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

Only win here!

Comment 114

[(no longer active)]

Birunthan, did you ever get a chance to test these patches on try?

Comment 115

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/layers/ipc/

Rebased patch.

Comment 116

[Biru [:poiru]]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #114)
> Birunthan, did you ever get a chance to test these patches on try?

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=c4ea56e76b84

Comment 117

[(no longer active)]

Thanks a lot for your patches!

https://hg.mozilla.org/integration/mozilla-inbound/rev/4c94544e2c09
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e44dee1202
https://hg.mozilla.org/integration/mozilla-inbound/rev/3638943416ab
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9b535ce91ae
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa06df37ebba
https://hg.mozilla.org/integration/mozilla-inbound/rev/59fed755d497
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecfdc57dacbd
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9e553a7232a
https://hg.mozilla.org/integration/mozilla-inbound/rev/f73954b225cc
https://hg.mozilla.org/integration/mozilla-inbound/rev/730fd7affc04
https://hg.mozilla.org/integration/mozilla-inbound/rev/a36bd2e1c291
https://hg.mozilla.org/integration/mozilla-inbound/rev/81b78a2ca769

Comment 118

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4c94544e2c09
https://hg.mozilla.org/mozilla-central/rev/e5e44dee1202
https://hg.mozilla.org/mozilla-central/rev/3638943416ab
https://hg.mozilla.org/mozilla-central/rev/e9b535ce91ae
https://hg.mozilla.org/mozilla-central/rev/aa06df37ebba
https://hg.mozilla.org/mozilla-central/rev/59fed755d497
https://hg.mozilla.org/mozilla-central/rev/ecfdc57dacbd
https://hg.mozilla.org/mozilla-central/rev/d9e553a7232a
https://hg.mozilla.org/mozilla-central/rev/f73954b225cc
https://hg.mozilla.org/mozilla-central/rev/730fd7affc04
https://hg.mozilla.org/mozilla-central/rev/a36bd2e1c291
https://hg.mozilla.org/mozilla-central/rev/81b78a2ca769

Comment 119

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/thebes/

Comment 120

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in mfbt/

Comment 121

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in parser/

Comment 122

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in profile/

Comment 123

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in rdf/

Comment 124

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in services/

Comment 125

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in startupcache/

Comment 126

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in storage/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=f9c8d4306072

Comment 127

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in mfbt/

Updated to fix compile errors on some platforms.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=48f5570c3b3d

Comment 128

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 780491 [details] [diff] [review]
Switch from NULL to nullptr in mfbt/

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

::: mfbt/RangedPtr.h
@@ +14,5 @@
>  
>  #include "mozilla/Assertions.h"
>  #include "mozilla/Attributes.h"
>  #include "mozilla/Util.h"
> +#include "mozilla/NullPtr.h"

NullPtr goes before Util

Comment 129

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in mfbt/

(In reply to :Ms2ger from comment #128)
> NullPtr goes before Util

Fixed, thanks!

Comment 130

[Jeff Walden [:Waldo]]

Comment on attachment 780552 [details] [diff] [review]
Switch from NULL to nullptr in mfbt/

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

Looks good with the one nit fixed.  Do you need someone to push this?  I can make that fix and push this, if you want, or you can post an appropriately revised patch and someone else can push it, if you'd prefer.

::: mfbt/tests/TestPoisonArea.cpp
@@ +195,5 @@
>    FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
>                   FORMAT_MESSAGE_FROM_SYSTEM |
>                   FORMAT_MESSAGE_IGNORE_INSERTS,
> +                 nullptr, errcode, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
> +                 (LPSTR)&errmsg, 0, nullptr);

This header doesn't #include "mozilla/NullPtr.h".  It needs to #include that for this to be okay.  Right now, you're probably only succeeding because you're on a compiler with nullptr support.

Comment 131

[Biru [:poiru]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #130)
> ... I can make that fix and push this ...

Sure, that'd be great. Thanks!

Comment 132

[Jeff Walden [:Waldo]]

Got it.  Queued up for the next time I have stuff to push.

Comment 133

[(no longer active)]

Comment on attachment 780473 [details] [diff] [review]
Switch from NULL to nullptr in storage/

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

::: storage/src/TelemetryVFS.cpp
@@ +393,2 @@
>      // so we always define them.  Verify that we're not going to call
> +    // nullptr pointers, though.

Nit: s/nullptr pointers/nullptrs/

Comment 134

[Jeff Walden [:Waldo]]

Pushed the mfbt/ changes, with minor post-review tweaks I noticed while double-checking |hg o -p|:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c10c0a6270ec

Thanks!

Comment 135

[Ryan VanderMeulen [:RyanVM]]

Something in this push made Windows checktests mad. Backed out per our IRC conversation.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbf37166d07c

Comment 136

[Jeff Walden [:Waldo]]

I fired up a Windows build, and the jsapi-tests executable (which failed in tbpl logs, although nobody noticed it just at the time) was crashing in PRMJ_ShutdownNow() code, or something similar, clearly implicating the JS_Init changes -- not this bug.  Relanded:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e978bb426e87

Comment 137

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e978bb426e87

Comment 138

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in gfx/thebes/

Rebased patch.

Comment 139

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in startupcache/

Rebased patch.

Comment 140

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in storage/

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #133)
> Nit: s/nullptr pointers/nullptrs/

Fixed.

Comment 141

[(no longer active)]

Landed all of the remaining patches (I think).  Please let me know if I missed something.

https://hg.mozilla.org/integration/mozilla-inbound/rev/71c2dd22bcca
https://hg.mozilla.org/integration/mozilla-inbound/rev/079bcec8ffd9
https://hg.mozilla.org/integration/mozilla-inbound/rev/88130a1e2831
https://hg.mozilla.org/integration/mozilla-inbound/rev/5172d6907d40
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac758cadd034
https://hg.mozilla.org/integration/mozilla-inbound/rev/417f4d51eba2
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfd50d4b9ac6

Comment 142

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/71c2dd22bcca
https://hg.mozilla.org/mozilla-central/rev/079bcec8ffd9
https://hg.mozilla.org/mozilla-central/rev/88130a1e2831
https://hg.mozilla.org/mozilla-central/rev/5172d6907d40
https://hg.mozilla.org/mozilla-central/rev/ac758cadd034
https://hg.mozilla.org/mozilla-central/rev/417f4d51eba2
https://hg.mozilla.org/mozilla-central/rev/dfd50d4b9ac6

Comment 143

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in browser/components/

Comment 144

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in browser/metro/

Comment 145

[Biru [:poiru]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=71c6447ede08

Comment 146

[(no longer active)]

Hey Birunthan, did I ever mention how much your work here is appreciated?! :-)  Thanks a lot!

Comment 147

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0c72a444eec
https://hg.mozilla.org/integration/mozilla-inbound/rev/15ce5693e28a

Comment 148

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c0c72a444eec
https://hg.mozilla.org/mozilla-central/rev/15ce5693e28a

Comment 149

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in image/

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #146)
> Hey Birunthan, did I ever mention how much your work here is appreciated?!
> :-)  Thanks a lot!

Glad to help.

Comment 150

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in hal/

Comment 151

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in intl/

Comment 152

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in ipc/

Comment 153

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in ipc/glue/

Comment 154

[Biru [:poiru]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=c1244ec4535f

Comment 155

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 793483 [details] [diff] [review]
Switch from NULL to nullptr in browser/components/

These aren't obsolete just because they were already checked in.

Comment 156

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7db2236a3775
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c8591a9369a
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd99696eb53d
https://hg.mozilla.org/integration/mozilla-inbound/rev/b25a650ea032
https://hg.mozilla.org/integration/mozilla-inbound/rev/21f216a5e2f1

Comment 157

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7db2236a3775
https://hg.mozilla.org/mozilla-central/rev/5c8591a9369a
https://hg.mozilla.org/mozilla-central/rev/fd99696eb53d
https://hg.mozilla.org/mozilla-central/rev/b25a650ea032
https://hg.mozilla.org/mozilla-central/rev/21f216a5e2f1

Comment 158

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in netwerk/

Comment 159

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/ductwork/, js/ipc/, and js/public/

I overzealously added `#include "mozilla/NullPtr.h"` to all public JS headers that I touched. Without it, B2G builds failed since the GCC version they use lacks support for nullptr. If you want, I can certainly look into removing the #include except where absolutely necessary.

Comment 160

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/jsd/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=bd1b8fddd36c

Comment 161

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in netwerk/ (1/2)

The try link in the previous comment was wrong. The correct url is: https://tbpl.mozilla.org/?tree=Try&rev=6a81463451c3

Comment 162

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in netwerk/ (2/2)

Comment 163

[(no longer active)]

(In reply to Birunthan Mohanathas [:poiru] from comment #159)
> Created attachment 807199 [details] [diff] [review]
> Switch from NULL to nullptr in js/ductwork/, js/ipc/, and js/public/
> 
> I overzealously added `#include "mozilla/NullPtr.h"` to all public JS
> headers that I touched. Without it, B2G builds failed since the GCC version
> they use lacks support for nullptr. If you want, I can certainly look into
> removing the #include except where absolutely necessary.

No, that's fine!

Comment 164

[(no longer active)]

Comment on attachment 807204 [details] [diff] [review]
Switch from NULL to nullptr in netwerk/ (1/2)

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

::: netwerk/build/nsNetModule.cpp
@@ +306,5 @@
>                                                        \
>    BaseWebSocketChannel * inst;                        \
>                                                        \
> +  *aResult = nullptr;                                    \
> +  if (nullptr != aOuter) {                               \

Nit: indentation (will fix when landing)

Comment 165

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a84f156f3164
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b174183996f
https://hg.mozilla.org/integration/mozilla-inbound/rev/06025e91dc9f
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae180b16eda7

Comment 166

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a84f156f3164
https://hg.mozilla.org/mozilla-central/rev/0b174183996f
https://hg.mozilla.org/mozilla-central/rev/06025e91dc9f
https://hg.mozilla.org/mozilla-central/rev/ae180b16eda7

Comment 167

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/builtin/

Comment 168

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ctypes/

Comment 169

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/{devtools,ds,gdb,perf}/

Comment 170

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/frontend/

Comment 171

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/gc/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=24238400cecd

Comment 172

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/builtin/

Rebased patch.

Comment 173

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/frontend/

Rebased patch.

Comment 174

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/gc/

Rebased patch.

Comment 175

[(no longer active)]

Luke, do I have your blessing to land these JS engine patches here?

Comment 176

[Luke Wagner [:luke]]

I think we should land the patches.  The biggest reason I see is that most (all?) of Gecko is doing it and consistency is good, but I'm also like that it reduces SHOUTCAPS.  These two things seem to outweigh the loss in terseness.

Before landing the js/src patches, though, could you please send an "I'm planning to make this change; here's why; any violent objections?" to m.d.t.js-engine.internals.  It'd be nice not to surprise everyone.

Comment 177

[(no longer active)]

(In reply to Luke Wagner [:luke] from comment #176)
> Before landing the js/src patches, though, could you please send an "I'm
> planning to make this change; here's why; any violent objections?" to
> m.d.t.js-engine.internals.  It'd be nice not to surprise everyone.

Sure, done.

Comment 178

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a28d2e865e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d34afe94163
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b4ab192b289
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4aa9fdddb72
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce4978329418

Comment 179

[:Benjamin Peterson]

Thanks, but I think you missed js/src/vm/.

Comment 180

[:Benjamin Peterson]

Also, js/src/jit.

Comment 181

[Biru [:poiru]]

(In reply to :Benjamin Peterson from comment #179)
> Thanks, but I think you missed js/src/vm/.

(In reply to :Benjamin Peterson from comment #180)
> Also, js/src/jit.

I have patches ready for them, but I have yet to review the patches myself. I also need to make sure they compile on B2G builds (which lack compiler support for nullptr). I'll try to do that in the evening (i.e. tomorrow morning MV time).

Comment 182

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0a28d2e865e4
https://hg.mozilla.org/mozilla-central/rev/5d34afe94163
https://hg.mozilla.org/mozilla-central/rev/0b4ab192b289
https://hg.mozilla.org/mozilla-central/rev/c4aa9fdddb72
https://hg.mozilla.org/mozilla-central/rev/ce4978329418

Comment 183

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (1/7)

Comment 184

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (2/7)

Comment 185

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (3/7)

Comment 186

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (4/7)

Comment 187

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (5/7)

Comment 188

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (6/7)

Comment 189

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ (7/7)

Comment 190

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jit/ subdirectories

Pushed js/src/jit/ changes to try: https://tbpl.mozilla.org/?tree=Try&rev=154672842bef

Comment 191

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/vm/ (1/5)

Comment 192

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/vm/ (2/5)

Comment 193

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/vm/ (3/5)

Comment 194

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/vm/ (3/5)

Comment 195

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/vm/ (4/5)

Comment 196

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/vm/ (5/5)

Pushed the js/src/vm/ changes to try: https://tbpl.mozilla.org/?tree=Try&rev=d23bdcbe911d

Apologies for exhausting you! :-)

Comment 197

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5761d3de664c
https://hg.mozilla.org/integration/mozilla-inbound/rev/c102b5ec30df
https://hg.mozilla.org/integration/mozilla-inbound/rev/e44494d17d37
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c0ec323b661
https://hg.mozilla.org/integration/mozilla-inbound/rev/59c9b78bef2c
https://hg.mozilla.org/integration/mozilla-inbound/rev/307c8a99f7eb
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7e1f15c2964
https://hg.mozilla.org/integration/mozilla-inbound/rev/bef1a1ec6ea3
https://hg.mozilla.org/integration/mozilla-inbound/rev/e34fa34cc870
https://hg.mozilla.org/integration/mozilla-inbound/rev/c793f516edda
https://hg.mozilla.org/integration/mozilla-inbound/rev/79a3d1d2c065
https://hg.mozilla.org/integration/mozilla-inbound/rev/d32249ad7115
https://hg.mozilla.org/integration/mozilla-inbound/rev/37cac3701742

Comment 198

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5761d3de664c
https://hg.mozilla.org/mozilla-central/rev/c102b5ec30df
https://hg.mozilla.org/mozilla-central/rev/e44494d17d37
https://hg.mozilla.org/mozilla-central/rev/0c0ec323b661
https://hg.mozilla.org/mozilla-central/rev/59c9b78bef2c
https://hg.mozilla.org/mozilla-central/rev/307c8a99f7eb
https://hg.mozilla.org/mozilla-central/rev/b7e1f15c2964
https://hg.mozilla.org/mozilla-central/rev/bef1a1ec6ea3
https://hg.mozilla.org/mozilla-central/rev/e34fa34cc870
https://hg.mozilla.org/mozilla-central/rev/c793f516edda
https://hg.mozilla.org/mozilla-central/rev/79a3d1d2c065
https://hg.mozilla.org/mozilla-central/rev/d32249ad7115
https://hg.mozilla.org/mozilla-central/rev/37cac3701742

Comment 199

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (1/9)

Comment 200

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (2/9)

Comment 201

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (3/9)

Comment 202

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (4/9)

Comment 203

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (5/9)

Comment 204

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (6/9)

Comment 205

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (7/9)

Comment 206

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (8/9)

Comment 207

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/ (9/9)

Comment 208

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/jsapi-tests/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=14386a2261fd / https://tbpl.mozilla.org/?tree=Try&rev=0e99c2f8e079

Comment 209

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0382512480b
https://hg.mozilla.org/integration/mozilla-inbound/rev/59c078a58301
https://hg.mozilla.org/integration/mozilla-inbound/rev/75f90df83f53
https://hg.mozilla.org/integration/mozilla-inbound/rev/e34c99c7dee4
https://hg.mozilla.org/integration/mozilla-inbound/rev/4177ead919e6
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce9cca8500d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/421efbf3ad86
https://hg.mozilla.org/integration/mozilla-inbound/rev/212dafcf376f
https://hg.mozilla.org/integration/mozilla-inbound/rev/09d8891fff22
https://hg.mozilla.org/integration/mozilla-inbound/rev/c131a717180e

Comment 210

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/c0382512480b
https://hg.mozilla.org/mozilla-central/rev/59c078a58301
https://hg.mozilla.org/mozilla-central/rev/75f90df83f53
https://hg.mozilla.org/mozilla-central/rev/e34c99c7dee4
https://hg.mozilla.org/mozilla-central/rev/4177ead919e6
https://hg.mozilla.org/mozilla-central/rev/ce9cca8500d5
https://hg.mozilla.org/mozilla-central/rev/421efbf3ad86
https://hg.mozilla.org/mozilla-central/rev/212dafcf376f
https://hg.mozilla.org/mozilla-central/rev/09d8891fff22
https://hg.mozilla.org/mozilla-central/rev/c131a717180e

Comment 211

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in layout/

Comment 212

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/gtk/

Comment 213

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/os2/

Comment 214

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/

Comment 215

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/windows/ (1/3)

Comment 216

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/windows/ (2/3)

Comment 217

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/windows/ (3/3)

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=3eac0e9dc377

The B2G ICS build failed due to an unrelated reason. I can retry that if needed.

Comment 218

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8cc13e82d47c
https://hg.mozilla.org/integration/mozilla-inbound/rev/36d4de5601b4
https://hg.mozilla.org/integration/mozilla-inbound/rev/89a576ec25cf
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ea080e6ae83
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7a720a0f63
https://hg.mozilla.org/integration/mozilla-inbound/rev/19c28d2f1c18

Comment 219

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8cc13e82d47c
https://hg.mozilla.org/mozilla-central/rev/36d4de5601b4
https://hg.mozilla.org/mozilla-central/rev/89a576ec25cf
https://hg.mozilla.org/mozilla-central/rev/4ea080e6ae83
https://hg.mozilla.org/mozilla-central/rev/1e7a720a0f63
https://hg.mozilla.org/mozilla-central/rev/19c28d2f1c18

Comment 220

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/xpconnect/

Comment 221

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in toolkit/

Comment 222

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in toolkit/components/ (1/2)

Comment 223

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in toolkit/components/ (2/2)

Comment 224

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in toolkit/crashreporter/

Comment 225

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in toolkit/mozapps/

Comment 226

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in xpcom/ (1/3)

Comment 227

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in xpcom/ (2/3)

Comment 228

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in xpcom/ (3/3)

Comment 229

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/android/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=0a14596e94ff

Comment 230

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ee0459d7336
https://hg.mozilla.org/integration/mozilla-inbound/rev/01fa49869175
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b9687f6c602
https://hg.mozilla.org/integration/mozilla-inbound/rev/62c48f864545
https://hg.mozilla.org/integration/mozilla-inbound/rev/18fa6912a68a
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc9733225130
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec55161f8059
https://hg.mozilla.org/integration/mozilla-inbound/rev/20f2a3e61f37
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad59ee55f596
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbae15cadd74

Comment 231

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9ee0459d7336
https://hg.mozilla.org/mozilla-central/rev/01fa49869175
https://hg.mozilla.org/mozilla-central/rev/8b9687f6c602
https://hg.mozilla.org/mozilla-central/rev/62c48f864545
https://hg.mozilla.org/mozilla-central/rev/18fa6912a68a
https://hg.mozilla.org/mozilla-central/rev/fc9733225130
https://hg.mozilla.org/mozilla-central/rev/ec55161f8059
https://hg.mozilla.org/mozilla-central/rev/20f2a3e61f37
https://hg.mozilla.org/mozilla-central/rev/ad59ee55f596
https://hg.mozilla.org/mozilla-central/rev/cbae15cadd74

Comment 232

[Bill Gianopoulos [:WG9s]]

This results in build failures under gcc 4.5.3.

toolkit/crashreporter/client/crashreporter.cpp:37:38: error: 'nullptr' was not declared in this scope

Comment 233

[Frank Wein [:mcsmurf]]

For reference: This checkin http://hg.mozilla.org/mozilla-central/rev/fc9733225130 (from this bug here) uncovered  Bug 926083, which is about libxul link step failing on Windows with MSVC 2012. The important bit is "uncovered" here as I think the actual problem was already there before the change to nullptr.

Comment 234

[Gregory Szorc [:gps]]

Apparently there are a number of developers using VS2012, especially in Metro land. They aren't taking too kindly to their builds being broken.

So, I backed out fc9733225130, which was isolated in comment #233 as the regression point. If we need to back out more changesets, a loud chorus of Windows developers would support that.

FWIW, I don't like backing out green changesets. But the reality is many Windows developers rely on VS2012. Bug 926705 has been filed to have RelEng deploy a VS2012 builder.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8bd4c964967

Comment 235

[Gregory Szorc [:gps]]

Some context here. Apparently way back in the day, VS2012 was required to build Firefox for Metro. So, many of our Metro developers have 2012 installed. They could possibly all be talked into installing 2010.

Comment 237

[Bill Gianopoulos [:WG9s]]

(In reply to Gregory Szorc [:gps] from comment #235)
> Some context here. Apparently way back in the day, VS2012 was required to
> build Firefox for Metro. So, many of our Metro developers have 2012
> installed. They could possibly all be talked into installing 2010.

Well yes but I thought later versions are required to build 64-bit Windows so probably still an issue for people trying to get that working.

Comment 238

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Gregory Szorc [:gps] from comment #235)
> Some context here. Apparently way back in the day, VS2012 was required to
> build Firefox for Metro. So, many of our Metro developers have 2012
> installed. They could possibly all be talked into installing 2010.

vs2012 is much nicer to use than vs2010, when the build isn't broken that is, so it would be a hard sell.

Comment 239

[Allison Naaktgeboren :ally]

(In reply to Chris Pearce (:cpearce) from comment #238)
> (In reply to Gregory Szorc [:gps] from comment #235)
> > Some context here. Apparently way back in the day, VS2012 was required to
> > build Firefox for Metro. So, many of our Metro developers have 2012
> > installed. They could possibly all be talked into installing 2010.
> 
> vs2012 is much nicer to use than vs2010, when the build isn't broken that
> is, so it would be a hard sell.

Since vs2010 was also busted by that changeset, so it would be a hard sell indeed. :)

Comment 240

[Gregory Szorc [:gps]]

(In reply to :Ally Naaktgeboren from comment #239)
> Since vs2010 was also busted by that changeset, so it would be a hard sell
> indeed. :)

We build with vs2010 in automation *today*. So, something about the environments is different. Different Windows SDK versions or a conflicting SDK, perhaps? Who knows. Just another reason why we need reproducible build environments for developers.

Comment 241

[Bill Gianopoulos [:WG9s]]

(In reply to :Ally Naaktgeboren from comment #239)
> (In reply to Chris Pearce (:cpearce) from comment #238)
> > (In reply to Gregory Szorc [:gps] from comment #235)
> > > Some context here. Apparently way back in the day, VS2012 was required to
> > > build Firefox for Metro. So, many of our Metro developers have 2012
> > > installed. They could possibly all be talked into installing 2010.
> > 
> > vs2012 is much nicer to use than vs2010, when the build isn't broken that
> > is, so it would be a hard sell.
> 
> Since vs2010 was also busted by that changeset, so it would be a hard sell
> indeed. :)

I built just fine under vs2010 with that changeset, albeit under windows 7 but did include metro in the build.

Comment 242

[(no longer active)]

Relanded the backed out changeset together with a workaround for bug 926083:

https://hg.mozilla.org/integration/mozilla-inbound/rev/73ac7e81f316

Comment 243

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/73ac7e81f316

Comment 244

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in content/

Comment 245

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in dom/plugins/base/

Comment 246

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in dom/plugins/ipc/

Comment 247

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in dom/plugins/test/

Comment 248

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/

Comment 249

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in miscellaneous directories

Comment 250

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in modules/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=e08fdb1745fc / https://tbpl.mozilla.org/?tree=Try&rev=a5372c0244c0

Comment 251

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b8c97df0418d
https://hg.mozilla.org/integration/mozilla-inbound/rev/3656e6195ed2
https://hg.mozilla.org/integration/mozilla-inbound/rev/298d6746f0af
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8b4feaa9add
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2540bc9686e
https://hg.mozilla.org/integration/mozilla-inbound/rev/58652b0ac86e
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7c32ea628e2

Comment 252

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b8c97df0418d
https://hg.mozilla.org/mozilla-central/rev/3656e6195ed2
https://hg.mozilla.org/mozilla-central/rev/298d6746f0af
https://hg.mozilla.org/mozilla-central/rev/d8b4feaa9add
https://hg.mozilla.org/mozilla-central/rev/e2540bc9686e
https://hg.mozilla.org/mozilla-central/rev/58652b0ac86e
https://hg.mozilla.org/mozilla-central/rev/b7c32ea628e2

Comment 253

[Gian-Carlo Pascutto [:gcp]]

...and gcc 4.5 is broken yet again. Please don't r+ patches like this unless they properly use mfbt/NullPtr.h, it's there for a reason.

Comment 254

[(no longer active)]

(In reply to comment #253)
> ...and gcc 4.5 is broken yet again. Please don't r+ patches like this unless
> they properly use mfbt/NullPtr.h, it's there for a reason.

Finding out where that include is actually needed requires somebody builds with gcc4.5 *on the platform that you are on*, which is not a supported build environment anyway.  Anyways, you have blanket r=me to add those includes anywhere that is needed until we figure out a better way (bug 930675 perhaps.)

Comment 255

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in dom/ (1/2)

Comment 256

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in dom/ (2/2)

Comment 257

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in security/

Comment 258

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in widget/

Comment 259

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=43a61654f7d3

Comment 260

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28e8a3bbe870
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bbd0f0eee60
https://hg.mozilla.org/integration/mozilla-inbound/rev/678df5249db9
https://hg.mozilla.org/integration/mozilla-inbound/rev/165922bc5650
https://hg.mozilla.org/integration/mozilla-inbound/rev/f19037548095

Comment 261

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/28e8a3bbe870
https://hg.mozilla.org/mozilla-central/rev/7bbd0f0eee60
https://hg.mozilla.org/mozilla-central/rev/678df5249db9
https://hg.mozilla.org/mozilla-central/rev/165922bc5650
https://hg.mozilla.org/mozilla-central/rev/f19037548095

Comment 262

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in build/

Comment 263

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in mozglue/

Comment 264

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in mozglue/linker/

Comment 265

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in tools/profiler/

Comment 266

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in miscellaneous directories

Comment 267

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/365669affac7
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccca3c4247e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c947073f4ea
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fc828acb653
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca67a11bfdcf

Comment 268

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/365669affac7
https://hg.mozilla.org/mozilla-central/rev/ccca3c4247e1
https://hg.mozilla.org/mozilla-central/rev/8c947073f4ea
https://hg.mozilla.org/mozilla-central/rev/0fc828acb653
https://hg.mozilla.org/mozilla-central/rev/ca67a11bfdcf

Comment 269

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in dom/indexedDB/

Comment 270

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in miscellaneous directories

Comment 271

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in webrtc/signaling/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=2ca89a3958b3

(Note that the indexedDB changes are included in the "miscellaneous directories" changeset in the try push above.)

Comment 272

[Ethan Hugg [:ehugg]]

Comment on attachment 8337264 [details] [diff] [review]
Switch from NULL to nullptr in webrtc/signaling/

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

Seems to still build and work fine.  

I do wonder if we should be changing lines like this:
if (a != NULL)
to:
if (a)
instead of:
if (a != nullptr)

Comment 273

[(no longer active)]

(In reply to Ethan Hugg [:ehugg] from comment #272)
> Seems to still build and work fine.  
> 
> I do wonder if we should be changing lines like this:
> if (a != NULL)
> to:
> if (a)
> instead of:
> if (a != nullptr)

I'd prefer if we did that in another bug, since this patch doesn't really change the weirdness of such code... :-)

Comment 274

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/525c49113540
https://hg.mozilla.org/integration/mozilla-inbound/rev/df7dd1ef49db
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b28c258bc84

Comment 275

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/525c49113540
https://hg.mozilla.org/mozilla-central/rev/df7dd1ef49db
https://hg.mozilla.org/mozilla-central/rev/8b28c258bc84

Comment 276

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in remaining directories

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=d7001fbeaf96

Comment 277

[Biru [:poiru]]

Attachment: Switch from NULL to nullptr in js/src/devtools/vprof/ and tools/{jprof,trace-malloc}/

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=a486edf8b46d

This should cover the last remaining instances of NULL in our own code. Unless I have missed something, I think this bug can be (finally) closed.

Comment 278

[(no longer active)]

Thanks a lot for your effort here, Birunthan!

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ebd1e1a8197
https://hg.mozilla.org/integration/mozilla-inbound/rev/5954f2857de4

Comment 279

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7ebd1e1a8197
https://hg.mozilla.org/mozilla-central/rev/5954f2857de4

Comment 280

[Omair Majid]

Comment on attachment 821220 [details] [diff] [review]
Switch from NULL to nullptr in dom/plugins/base/

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

::: dom/plugins/base/npruntime.h
@@ +136,5 @@
>  
>  #define VOID_TO_NPVARIANT(_v)                                                 \
>  NP_BEGIN_MACRO                                                                \
>      (_v).type = NPVariantType_Void;                                           \
> +    (_v).value.objectValue = nullptr;                                         \

This file is a public API header. Various NPAPI plugins (which are not part of Mozilla) include this file.

Do you really want to require that anything using NPAPI must be built with C++11 support?

Comment 281

[(no longer active)]

(In reply to Omair Majid from comment #280)
> Comment on attachment 821220 [details] [diff] [review]
> Switch from NULL to nullptr in dom/plugins/base/
> 
> Review of attachment 821220 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/plugins/base/npruntime.h
> @@ +136,5 @@
> >  
> >  #define VOID_TO_NPVARIANT(_v)                                                 \
> >  NP_BEGIN_MACRO                                                                \
> >      (_v).type = NPVariantType_Void;                                           \
> > +    (_v).value.objectValue = nullptr;                                         \
> 
> This file is a public API header. Various NPAPI plugins (which are not part
> of Mozilla) include this file.
> 
> Do you really want to require that anything using NPAPI must be built with
> C++11 support?

No, that was a mistake, thanks for catching it.  Fixed in https://hg.mozilla.org/integration/mozilla-inbound/rev/0691948a0abe.

Comment 282

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0691948a0abe

Comment 283

[Gabriele Giacone]

I was told it's not possible to get it backported to 27.*. Any chance to have it in 28? Thanks

Comment 284

[(no longer active)]

(In reply to Gabriele Giacone from comment #283)
> I was told it's not possible to get it backported to 27.*. Any chance to
> have it in 28? Thanks

What is it that you want backported exactly?

Comment 285

[Gabriele Giacone]

(In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, emailapocalypse) from comment #284)
> (In reply to Gabriele Giacone from comment #283)
> > I was told it's not possible to get it backported to 27.*. Any chance to
> > have it in 28? Thanks
> 
> What is it that you want backported exactly?

Switching back to NULL for plugins.
https://hg.mozilla.org/mozilla-central/rev/0691948a0abe
Gnash plugin build against 27 is broken.

Comment 286

[(no longer active)]

Attachment: Fix the npapi headers

Note about this approval request: this patch doesn't change anything in the binaries that we ship, so it's 100% safe to backport anywhere.  It however changes the NPAPI headers we ship as part of the SDK which people use to build plugins. based on to make them C compatible again.  Without this patch, the plugin developers will need to fix this header themselves every time.

The mozilla-release approval is made so that this gets included in a potential next dot-release of 27 (in the SDK accompanying it to be more precise.)

Comment 287

[Benjamin Smedberg]

The NPAPI headers should be unmodified copies of the files from the npapi-sdk project: https://code.google.com/p/npapi-sdk/

Is that still the case? (Plugin developers should be using those headers instead of the Mozilla SDK anyway).

Comment 288

[(no longer active)]

(In reply to comment #287)
> The NPAPI headers should be unmodified copies of the files from the npapi-sdk
> project: https://code.google.com/p/npapi-sdk/
> 
> Is that still the case? (Plugin developers should be using those headers
> instead of the Mozilla SDK anyway).

Not sure what they use, but we definitely incorrectly modified the ones in our tree.

Comment 289

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8380682 [details] [diff] [review]
Fix the npapi headers

No sign yet that there would be a 27.0.2 - approving for beta/aurora though and will keep the nom up for release in case the unexpected happens.

Comment 290

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/939571a11ebb
https://hg.mozilla.org/releases/mozilla-beta/rev/85053eae528b

Note that the "status-firefox28:fixed" status only applies to the very specific NPAPI follow-up fix in comment 282, not to the entirety of what landed over the course of this bug. It would have been preferable if this follow-up work be done in a separate bug for tracking purposes, but that's neither here nor there now, I guess.

Comment 291

[u279076]

This seems like it has a broad range of impact. Could someone please advise QA what, if any, areas of potential regression exist so we can test this before it is released?

Comment 292

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/85053eae528b

Comment 293

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This doesn't need manual QA.

Comment 294

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8380682 [details] [diff] [review]
Fix the npapi headers

It is in 29 and we are going to release it next week...