Closed
Bug 132949
Opened 22 years ago
Closed 19 years ago
Uninitialized pointer used in function call in jsfile.c
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: davidmaynor, Assigned: mrbkap)
References
Details
(Keywords: fixed1.8, js1.5)
Attachments
(1 file, 5 obsolete files)
54.55 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
In the function "file_currentDirSetter" the JSObject "rhsObject" is used in multiple functions (see below). Should be initialized to "obj" or all "rhsObject" should be changed to "obj". static JSBool file_currentDirSetter(JSContext *cx, JSObject *obj, jsval id, jsval *vp) { JSObject *rhsObject; /* should be = obj */ char *path; JSFile *file = JS_GetInstancePrivate(cx, rhsObject, &file_class, NULL); /* Look at the rhs and extract a file object from it */ if (JSVAL_IS_OBJECT(*vp)){ if (JS_InstanceOf(cx, rhsObject, &file_class, NULL)){ ... }
Comment 1•22 years ago
|
||
Formally confirming for consideration. cc'ing Brendan on this one -
Assignee: rogerl → khanson
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•22 years ago
|
||
jsfile.c is not well-supported, but we should get this fix in for 1.0 and js1.5 -- Kenton, can you fix? Thanks, /be
Keywords: js1.5,
mozilla1.0
Reporter | ||
Comment 3•22 years ago
|
||
I made the changes and tested this patch. There are a few other uninitialized variables in the code but not as likely to be called or to produce an error (depends on the compiler). Let me know if i should open other bug reports. Thanks david
Comment 4•22 years ago
|
||
Brendan, yes I will follow this. David, Can you make the patch into a diff file? If not, I can do it. =Kenton=
Reporter | ||
Comment 5•22 years ago
|
||
If there is a diff program (win32) you can point me to sure. Otherwise I'm out of luck -- I'm an old programmer new to open source development :) david
Comment 6•22 years ago
|
||
That looks like the whole file. Can you attach a cvs diff -u instead? Or diff -u if you're working without cvs for some reason. Feel free to fix all problems in this bug, no need to split bugs out yet (no real symptom-based reports yet). Thanks for doing this! /be
Comment 7•22 years ago
|
||
Argh, I did it again: I commented based on bugzilla mail notification before reading all the later comments in the bug. Anyhoo, you can get cvs for windows at http://www.cvshome.org/dev/codewindow.html and diff and other tools at http://sources.redhat.com/cygwin/download.html -- see http://www.mozilla.org/build/win32.html, where I found these links. /be
Comment 8•22 years ago
|
||
Comment 9•22 years ago
|
||
The patch above is a cvs diff -u of David's fix so far. As Brendan said above, if there are any other problems in this file, please feel free to add fixes for them, too. I can make another diff anytime, if you attach your jsfile.c to this bug -
Comment 10•22 years ago
|
||
A problem with a file that is not part of the default build of either mozilla or spidermonkey is not a blocker.
Severity: blocker → normal
Reporter | ||
Comment 11•22 years ago
|
||
This patch (diff -u and I hope I got it right) contains the original mentioned pointer and four others. The others are likely to not cause a problem but I figured why not fix them them anyway. oh and you're welcome Brendan -- anytime. david
Comment 12•22 years ago
|
||
Brendan or David, can you advise how to use/test this file? Thanks, Kenton
Reporter | ||
Comment 13•22 years ago
|
||
To test the patch the following script can be executed using the shell or another embedding againt a library compiled with "JS_HAS_FILE_OBJECT" : f = new File(); f.currentDir = new File("D:\\"); david
Comment 14•22 years ago
|
||
Tried David's suggestion on the MAC using Codewarrior with no success. I cuoldn't resovle all the links. I suspect jsfile support is supported only in the PC environment. Any comments or suggestions? I will try the patch using VC++.
Comment 15•22 years ago
|
||
Also note bug 181324, "jsfile.c buggy on Windows"
Comment 16•22 years ago
|
||
Comment on attachment 75799 [details] [diff] [review] Proposed patch with all found possibly used unitialized vars note that in jseng comments should be sentences, so + maybe not necessary */ is missing a period "." repeatedly. Also, CVS is responsible for versioning, so you don't need to include comments saying who you are and what you did. Also, freeing a null pointer isn't liked by certain CRTs... I'm temporarily claiming ownership of this bug. but if i don't fix it within a month it should be reset to the default owner (my attention span sucks). phil: i'm slowly working through jsengine bugs, but i haven't seen one about codewarrior not liking jsfile, do you know of one? (a quick look through the buglist i'm reading doesn't show one.) http://www.mozilla.org/js/js-file-object.html indicates that no one has tried to build it on macos... i'd say that should be covered in another bug :).
Assignee: khanson → timeless
Updated•19 years ago
|
QA Contact: pschwartau → general
Assignee | ||
Comment 17•19 years ago
|
||
I fixed some of the style problems in the functions pointed out in attachment 75799 [details] [diff] [review]. I still need to fix the one reported by e-mail, though. I'll do that soon.
Assignee: timeless → mrbkap
Attachment #75757 -
Attachment is obsolete: true
Attachment #75784 -
Attachment is obsolete: true
Attachment #75799 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Assignee | ||
Comment 18•19 years ago
|
||
Along with the uninitialized pointers and the poorly implemented js_fileDirectoryName, this fixes file_readln to work for lines greater than 256 characters in length. Shaver, you get the review request, feel free to take your time getting to this.
Assignee | ||
Updated•19 years ago
|
Attachment #191001 -
Attachment is obsolete: true
Attachment #191021 -
Flags: review?(shaver)
Assignee | ||
Updated•19 years ago
|
Attachment #191021 -
Flags: review?(brendan)
Assignee | ||
Updated•19 years ago
|
Attachment #191021 -
Attachment is obsolete: true
Attachment #191021 -
Flags: review?(shaver)
Attachment #191021 -
Flags: review?(brendan)
Attachment #191021 -
Flags: review+
Assignee | ||
Updated•19 years ago
|
Attachment #191021 -
Flags: review+
Assignee | ||
Comment 19•19 years ago
|
||
This patch keeps growing as I find stuff that just needs fixing. Sorry. This addresses a bunch of comments that Shaver made earlier. Note the changes to js_fileDirectoryName and file_readln.
Assignee | ||
Updated•19 years ago
|
Attachment #192241 -
Flags: review?(shaver)
Assignee | ||
Comment 20•19 years ago
|
||
*** Bug 181324 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 21•19 years ago
|
||
Comment on attachment 192241 [details] [diff] [review] more fixes shaver says on IRC: rs+a=him
Attachment #192241 -
Flags: review?(shaver) → review+
Assignee | ||
Comment 22•19 years ago
|
||
I checked attachment 192241 [details] [diff] [review] into the trunk and branch, so I'm marking this bug FIXED. There are more (probably lots more) bugs (and improvements) to be made in jsfile.c, but they should be handled in seperate bugs. I'll try to hack on them when I have time, but it's looking like every other bug on my list has higher priority. In the meantime, jsfile.c should be relatively safe to use, only stomping on bad memory in the odd case, instead of every case.
You need to log in
before you can comment on or make changes to this bug.
Description
•