Closed Bug 727156 Opened 12 years ago Closed 12 years ago

fix warnings in toolkit/mozapps/update/

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla12
Tracking Status
firefox11 --- unaffected
firefox12 --- fixed
firefox13 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

There's a couple different warnings here:

* set-but-unused variables
* unused functions (!)
* const char * -> char * warnings

Let's go ahead and fix them all in one go.
Attached patch patch (obsolete) — Splinter Review
Attachment #597089 - Flags: review?(netzen)
Comment on attachment 597089 [details] [diff] [review]
patch

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

Thanks for the patch.
But we need to tweak the ifdefs instead of removing those functions.

::: toolkit/mozapps/update/updater/updater.cpp
@@ -1461,5 @@
> - * @return true if the information was retrieved and it is pending
> - *         or pending-service.
> -*/
> -static bool
> -IsUpdateStatusPending(bool &isPendingService)

This is needed when #ifdef MOZ_MAINTENANCE_SERVICE (which happens currently on windows x86)

@@ -1495,5 @@
> - *         is set to succeeded or not.
> - * @return true if the information was retrieved and it is succeeded.
> -*/
> -static bool
> -IsUpdateStatusSucceeded(bool &isSucceeded)

This is needed on Windows so when #ifdef XP_WIN.
Attachment #597089 - Flags: review?(netzen) → review-
Attached patch patch (obsolete) — Splinter Review
Thanks for the pointers.  That's the fun of fixing compiler warnings, isn't it?
Attachment #597089 - Attachment is obsolete: true
Attachment #597109 - Flags: review?(netzen)
Comment on attachment 597109 [details] [diff] [review]
patch

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

One last pass pls :D
Also pls run through try to make sure it builds fine on x64 and Windows x86.

::: toolkit/mozapps/update/updater/updater.cpp
@@ +1522,2 @@
>  
>  #ifdef XP_WIN

The previous #ifdef XP_WIN can be combined with this one.
Attachment #597109 - Flags: review?(netzen)
Attached patch patchSplinter Review
Attachment #597109 - Attachment is obsolete: true
Whiteboard: [autoland-try]
Whiteboard: [autoland-try] → [autoland-in-queue]
Autoland Patchset:
	Patches: 597113
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=b19c8e48aeaf
Try run started, revision b19c8e48aeaf. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=b19c8e48aeaf
Comment on attachment 597113 [details] [diff] [review]
patch

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

Thanks for cleaning this up, you're good to go as long as it passes try.
Attachment #597113 - Flags: review+
Try run for b19c8e48aeaf is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b19c8e48aeaf
Results (out of 210 total builds):
    success: 177
    warnings: 19
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-b19c8e48aeaf
Whiteboard: [autoland-in-queue]
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a8634484e86b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: