276207 - Fixed possible buffer overflow + smaller cleanups

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Tim Ruehsen]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.3; Linux) (KHTML, like Gecko)
Build Identifier: 

- in function file_toURL: fixed possible buffer overflow + small cleanups 
- in function js_combinePath: replaced strcat by strcpy (bit faster + better 
readable) 
- in function file_getProperty: simplified case FILE_MODE for better 
readability, removed memory allocation 
 

Reproducible: Always

Comment 1

[Tim Ruehsen]

Attachment: cvs diff

Comment 2

[Brendan Eich [:brendan]]

jsfile.[ch] are not compiled into any Mozilla products, and they're pretty ugly,
but if you want to fix 'em, I'll mostly-rubberstamp your patches.

/be

Comment 3

[Tim Ruehsen]

I agree, that the code is a bit ugly. 
But i didn't realize that jsfile.[ch] isn't used. 
 
Then, why don't we kick it out of CVS? 
As far as I can see, we have to modify 
mozilla/js/src/Makefile.ref (jsfile.h, jsfile.c, JS_HAS_FILE_OBJECT) 
mozilla/js/src/js.mak (jsfile.h) 
mozilla/js/src/jsapi.c (jsfile.h, JS_HAS_FILE_OBJECT, js_InitFileClass) 
 
and remove 
mozilla/js/src/jsfile.h 
mozilla/js/src/jsfile.c 
mozilla/js/src/jsfile.msg 
 
I am willing to make a patch for the above modifications (just say Go!). 
But after putting that into CVS, someone else has to remove the named files. 
 
Did I miss something? 
 

Comment 4

[timeless]

err. he said they aren't used by *mozilla* products, that doesn't mean *no one*
uses them. you can build w/ jsfile if you like. (i do occasionally.)

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

No-one should be using jsfile, IMO, based on its history of neglect and lack of
good review.  We should remove it until an owner steps forward to maintain it,
and the need for it, in light of the presence of xpcshell and xpcom IO, is
justified (to the JS module owner, natch) against the cost of having it in the tree.

Comment 6

[Bob Clary [:bc] (inactive)]

-> default qa

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 169712 [details] [diff] [review]
cvs diff

Disowning, try mrbkap.

/be