Closed
Bug 834072
Opened 11 years ago
Closed 11 years ago
Fix build warnings in uriloader/exthandler/android
Categories
(Core Graveyard :: Widget: Android, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla21
People
(Reporter: dholbert, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
2.25 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #705645 -
Flags: review?(blassey.bugs)
Assignee | ||
Updated•11 years ago
|
Component: General → Widget: Android
OS: Linux → Android
Product: Fennec → Core
Hardware: x86_64 → All
Assignee | ||
Comment 3•11 years ago
|
||
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.
Assignee | ||
Comment 4•11 years ago
|
||
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)
Assignee | ||
Comment 5•11 years ago
|
||
(and since I want to land bug 834054 & this is the only thing blocking it)
Comment 6•11 years ago
|
||
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+
Assignee | ||
Comment 7•11 years ago
|
||
Good point -- thanks for the review!
Assignee | ||
Comment 8•11 years ago
|
||
Fixed that & landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/82256205e936
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/82256205e936
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•