Error when building purplexpcom

RESOLVED FIXED in 1.6

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sawrubh, Assigned: sawrubh)

Tracking

trunk
x86_64
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

5 years ago
I'm doing an optimized build (have explicitly specified --disable-debug in the mozconfig) but getting the following errors : http://pastebin.mozilla.org/5156688

With this diff (http://pastebin.mozilla.org/5156660), I get this error (http://pastebin.mozilla.org/5156673).
Assignee

Comment 1

5 years ago
Posted patch Patch(v1) (obsolete) — Splinter Review
This fixes the build issues.
Assignee: nobody → saurabhanandiit
Status: NEW → ASSIGNED
Attachment #8422578 - Flags: feedback?(florian)
Comment on attachment 8422578 [details] [diff] [review]
Patch(v1)

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

Thanks for looking into this!

It's tempting to do some other cleanups while touching this code, but in the interest of fixing the bustage sooner than later, I think we should do that in a follow-up.

::: purplexpcom/src/purpleInitConv.cpp
@@ +32,5 @@
>  static void create_conv(PurpleConversation *aConv)
>  {
>    LOG(("Create conversation with name: %s", purple_conversation_get_name(aConv)));
>  
> +  purpleConversation* conv;

nit: purpleConversation *conv;

@@ +49,5 @@
>    conv->SetConv(aConv);
>  
>    nsCOMPtr<imIConversationsService> convs =
>      do_GetService("@mozilla.org/chat/conversations-service;1");
>    NS_ENSURE_TRUE(convs, );

Please move these 3 lines before the |purpleConversation *conv;| line.

The reason for this change is that with an nsCOMPtr or nsRefPtr smart pointer, we will automatically release the object (which will cause it to be freed) if we return early before doing something with the conv object. With a raw pointer, this won't happen, so we need to ensure we are never going to return early.

@@ +83,5 @@
>  static void write_conv(PurpleConversation *conv, const char *who,
>                         const char *alias, const char *message,
>                         PurpleMessageFlags flags, time_t mtime)
>  {
> +  purpleMessage* msg = new purpleMessage();

nit: purpleMessage *msg

Note: I have no idea of why an nsRefPtr didn't work here :-S.
Attachment #8422578 - Flags: review-
Attachment #8422578 - Flags: feedback?(florian)
Attachment #8422578 - Flags: feedback+
Assignee

Comment 3

5 years ago
Posted patch Patch(v2)Splinter Review
Attachment #8422578 - Attachment is obsolete: true
Attachment #8422916 - Flags: review?(florian)
Comment on attachment 8422916 [details] [diff] [review]
Patch(v2)

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

Thanks!
Attachment #8422916 - Flags: review?(florian) → review+
https://hg.mozilla.org/users/florian_queze.net/purple/rev/cba9bdaf5ef6
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
Comment on attachment 8423157 [details] [diff] [review]
More fixes when debug is enabled

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

::: purplexpcom/src/purpleInitContacts.cpp
@@ +10,5 @@
>  #include <imIContactsService.h>
>  #include <nsServiceManagerUtils.h>
>  #include <nsCOMPtr.h>
>  #include <nsStringAPI.h>
> +#include "nsAutoPtr.h" // for nsRefPtr

Use <> rather than "" for headers that aren't expected to be in the same folder as the file you are compiling.

Also, this list is vaguely sorted alphabetically, so if that doesn't cause any issue, it would be better to add nsAutoPtr before nsCOMPtr.
Attachment #8423157 - Flags: review?(florian) → review+
You need to log in before you can comment on or make changes to this bug.