Closed Bug 608818 Opened 14 years ago Closed 14 years ago

nsFilePicker::Show uninitialised use of mAllowURLs


(Core :: Widget: Gtk, defect)

Not set





(Reporter: dougt, Assigned: dougt)



(2 files)

I saw this when testing creating a PDF in fennec:

Saving a PDF to disk, looks like we don't call AppendFilter.

==22492== Conditional jump or move depends on uninitialised value(s)
==22492==    at 0x6B0E51C: nsFilePicker::Show(short*) (nsFilePicker.cpp:453)
==22492==    by 0x6CA7E28: NS_InvokeByIndex_P (xptcinvoke_x86_64_unix.cpp:208)
==22492==    by 0x690628E: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (xpcwrappednative.cpp:3054)
==22492==    by 0x690BD60: XPC_WN_CallMethod(JSContext*, unsigned int, unsigned long*) (xpcwrappednativejsops.cpp:1631)
==22492==    by 0x6F8F50D: js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) (jscntxtinlines.h:684)
==22492==    by 0x6DC9008: js::Invoke(JSContext*, js::CallArgs const&, unsigned int) (jsinterp.cpp:665)
==22492==    by 0x6DC9967: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:881)
==22492==    by 0x6D5A6B0: JS_CallFunctionValue (jsinterp.h:954)
==22492==    by 0x6676D5F: nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) (nsJSEnvironment.cpp:2171)
==22492==    by 0x66B4CE9: nsJSEventListener::HandleEvent(nsIDOMEvent*) (nsJSEventListener.cpp:228)
==22492==    by 0x657DE5C: nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsPIDOMEventTarget*, unsigned int, nsCxPusher*) (nsEventListenerManager.cpp:1112)
==22492==    by 0x657E1F8: nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsPIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) (nsEventListenerManager.cpp:1208)
Attached patch patch v.1Splinter Review
Regression from:
Assignee: nobody → doug.turner
Attachment #487416 - Flags: review?
Attachment #487416 - Flags: review? → review?(timeless)
Comment on attachment 487416 [details] [diff] [review]
patch v.1

glanced over this at dougt's request -- looks fine to me.

AFAICT, we only would want to allow URLs in our FilePicker if it were specifically requested via AppendFilter().  So if AppendFilter() never gets called (as is the case here, per comment 0), then we *don't* want to allow URLs.  

So PR_FALSE makes sense as a default value for mAllowURLs.

r=dholbert, but you might technically need karl or roc (widget GTK owner/peer) to rubber-stamp this.
Attachment #487416 - Flags: review?(timeless) → review+
Attachment #487416 - Flags: review?(roc)
Hardware: x86_64 → All
Version: unspecified → Trunk
Comment on attachment 487416 [details] [diff] [review]
patch v.1

low risk - just sets default value to a valid state.
Attachment #487416 - Flags: approval2.0?
Attachment #487416 - Flags: approval2.0? → approval2.0+
Closed: 14 years ago
Resolution: --- → FIXED
(In comment 4, dougt meant to quote the checkin:)

Sadly, this patch was responsible for a silly build warning:
> gtk2/nsFilePicker.h: In constructor ‘nsFilePicker::nsFilePicker()’:
> gtk2/nsFilePicker.h:86: warning: ‘nsFilePicker::mAllowURLs’ will be initialized after
> gtk2/nsFilePicker.h:85: warning:   ‘PRInt16 nsFilePicker::mSelectedType’
> gtk2/nsFilePicker.cpp:195: warning:   when initialized here

(sorry for not having caught that in review)

Attaching trivial followup to fix this warning.
Attachment #506325 - Flags: review?(doug.turner)
Attachment #506325 - Flags: approval2.0?
Attachment #506325 - Attachment description: trivial followup to fix build warning → trivial followup to fix constructor init order
Attachment #506325 - Flags: review?(doug.turner)
Attachment #506325 - Flags: review+
Attachment #506325 - Flags: approval2.0?
Attachment #506325 - Flags: approval2.0+
You need to log in before you can comment on or make changes to this bug.