Closed Bug 1010263 Opened 8 years ago Closed 8 years ago

Error when building purplexpcom

Categories

(Chat Core :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sawrubh, Assigned: sawrubh)

Details

Attachments

(2 files, 1 obsolete file)

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).
Attached 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+
Attached 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: 8 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.