Closed Bug 834072 Opened 11 years ago Closed 11 years ago

Fix build warnings in uriloader/exthandler/android

Categories

(Core Graveyard :: Widget: Android, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla21

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

uriloader/exthandler/android currently has these build warnings:
{
../../../uriloader/exthandler/android/nsAndroidHandlerApp.h: In constructor 'nsAndroidHandlerApp::nsAndroidHandlerApp(const nsAString_internal&, const nsAString_internal&, const nsAString_internal&, const nsAString_internal&, const nsACString_internal&, const nsAString_internal&)':
../../../uriloader/exthandler/android/nsAndroidHandlerApp.h:30:14: warning: 'nsAndroidHandlerApp::mPackageName' will be initialized after [-Wreorder]
../../../uriloader/exthandler/android/nsAndroidHandlerApp.h:29:14: warning:   'nsString nsAndroidHandlerApp::mClassName' [-Wreorder]
../../../uriloader/exthandler/android/nsAndroidHandlerApp.cpp:12:1: warning:   when initialized here [-Wreorder]
../../../uriloader/exthandler/android/nsAndroidHandlerApp.h:29:14: warning: 'nsAndroidHandlerApp::mClassName' will be initialized after [-Wreorder]
../../../uriloader/exthandler/android/nsAndroidHandlerApp.h:28:15: warning:   'nsCString nsAndroidHandlerApp::mMimeType' [-Wreorder]
../../../uriloader/exthandler/android/nsAndroidHandlerApp.cpp:12:1: warning:   when initialized here [-Wreorder]
}
...and...
{
../../../uriloader/exthandler/android/nsExternalSharingAppService.cpp: In member function 'virtual nsresult nsExternalSharingAppService::GetSharingApps(const nsAString_internal&, uint32_t*, nsISharingHandlerApp***)':
../../../uriloader/exthandler/android/nsExternalSharingAppService.cpp:60:24: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
}
https://tbpl.mozilla.org/php/getParsedLog.php?id=19070108&full=1&branch=try

Filing this bug on fixing those.
We just need to use a uint32_t as the loop variable (matching the type of "*aLen", which we compare it to, and reorder the nsAndroidHandlerApp constructor's init list to match the .h file's order.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attached patch fixSplinter Review
Attachment #705645 - Flags: review?(blassey.bugs)
Component: General → Widget: Android
OS: Linux → Android
Product: Fennec → Core
Hardware: x86_64 → All
Comment on attachment 705645 [details] [diff] [review]
fix

># HG changeset patch
># Parent 8fda2cedebafd34065da06674b25fa1d67f9ae08
># User Daniel Holbert <dholbert@cs.stanford.edu>
>Bug 834072 - Fix build warnings in uriloader/exthandler/android. r=blassey
>
>diff --git a/uriloader/exthandler/android/nsAndroidHandlerApp.cpp b/uriloader/exthandler/android/nsAndroidHandlerApp.cpp
>--- a/uriloader/exthandler/android/nsAndroidHandlerApp.cpp
>+++ b/uriloader/exthandler/android/nsAndroidHandlerApp.cpp
>@@ -10,18 +10,22 @@
> NS_IMPL_ISUPPORTS2(nsAndroidHandlerApp, nsIHandlerApp, nsISharingHandlerApp)
> 
> nsAndroidHandlerApp::nsAndroidHandlerApp(const nsAString& aName,
>                                          const nsAString& aDescription,
>                                          const nsAString& aPackageName,
>                                          const nsAString& aClassName,
>                                          const nsACString& aMimeType,
>                                          const nsAString& aAction) :
>-mName(aName), mDescription(aDescription), mPackageName(aPackageName),
>-  mClassName(aClassName), mMimeType(aMimeType), mAction(aAction)
>+  mName(aName),
>+  mDescription(aDescription),
>+  mMimeType(aMimeType),
>+  mClassName(aClassName),
>+  mPackageName(aPackageName),
>+  mAction(aAction)

For reference, these member-variables are declared here:
12 class nsAndroidHandlerApp : public nsISharingHandlerApp {
[...]
25 private:
26     nsString mName;
27     nsString mDescription;
28     nsCString mMimeType;
29     nsString mClassName;
30     nsString mPackageName;
31     nsString mAction;
https://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/android/nsAndroidHandlerApp.h

As you can see, that ordering matches my post-patch init-list order in the quoted patch-chunk above, which makes the compiler happy.
Comment on attachment 705645 [details] [diff] [review]
fix

switching r? to kats, since I got his attention
Attachment #705645 - Flags: review?(blassey.bugs) → review?(bugmail.mozilla)
(and since I want to land bug 834054 & this is the only thing blocking it)
Comment on attachment 705645 [details] [diff] [review]
fix

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

r=me with the fields in nsAndroidHandlerApp.h reordered instead. That way the init list ordering stays the same as the argument list to the constructor and everything is consistent.
Attachment #705645 - Flags: review?(bugmail.mozilla) → review+
Good point -- thanks for the review!
https://hg.mozilla.org/mozilla-central/rev/82256205e936
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.