Open Bug 1113471 Opened 5 years ago Updated 3 years ago

OS.Constants.Win.GENERIC_READ is wrong

Categories

(Toolkit :: OS.File, defect)

35 Branch
x86_64
Windows 8.1
defect
Not set

Tracking

()

People

(Reporter: noitidart, Assigned: manan.doshi96, Mentored)

Details

(Whiteboard: [lang=c++][good first bug])

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141211142524

Steps to reproduce:

OS.Constants.Win.GENERIC_READ is coming back as -2147483648

It shouldbe though 0x80000000
Which is 2147483648
Component: Untriaged → OS.File
Product: Firefox → Toolkit
The right way to fix this is to change the definition of GENERIC_READ in http://dxr.mozilla.org/mozilla-central/source/dom/system/OSFileConstants.cpp to use `UINT_CONSTANT` instead of `INT_CONSTANT`.
Mentor: dteller
Whiteboard: [lang=c++][good first bug]
I'd like to work on it.
Attached patch generic_read.patch (obsolete) — Splinter Review
Made changes as suggested.
Attachment #8547138 - Flags: review?(dteller)
Comment on attachment 8547138 [details] [diff] [review]
generic_read.patch

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

Looks good, thanks.
Let's test that this doesn't break anything: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d61bb8c11e29

Also, can you use "r=yoric" at the end of your commit message?
Attachment #8547138 - Flags: review?(dteller) → review+
Assignee: nobody → manan.doshi96
How do I test it? (I am fairly new to this).
Added r=yoric
Attachment #8547138 - Attachment is obsolete: true
forgot to add a needinfo
Comment on attachment 8550843 [details] [diff] [review]
generic_read.patch

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

Looks good to me.
Try results will show up here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e410317d26a
Attachment #8550843 - Flags: review+
Tests look good. Are you ready to land this, Manan?
Flags: needinfo?(manan.doshi96)
Well, I'm going to assume that he is.
Flags: needinfo?(manan.doshi96)
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/c4915d92b0c5
Keywords: checkin-needed
Whiteboard: [lang=c++][good first bug] → [lang=c++][good first bug][fixed-in-fx-team]
Backed out for Windows test_osfile_back.xul failures.
https://hg.mozilla.org/integration/fx-team/rev/c218112d84f7

https://treeherder.mozilla.org/logviewer.html#?job_id=2341144&repo=fx-team
Whiteboard: [lang=c++][good first bug][fixed-in-fx-team] → [lang=c++][good first bug]
I don't understand how UINT_CONSTANT made the test_osfile_back.xul fail

Can someone please help me understand that, now that i got firefox built I was hoping to land a fix for this
David: do you know why changing INT_CONSTANT to UINT_CONSTANT broke the test_osfile_back.xul test on fxteam but not Try? From the test errors, it looks like Python choked when parsing ps' output:

14. 'c:/mozilla-build/python27/python -u ...' warnings 46m 30s 27ms
1637 10:43:16 INFO - ValueError: invalid literal for int() with base 10: 'ps:'
1651 10:43:16 INFO - ValueError: invalid literal for int() with base 10: 'ps:'
28235 11:00:05 INFO - 2066 INFO TEST-UNEXPECTED-FAIL | toolkit/components/osfile/tests/mochi/test_osfile_back.xul | Worker error TypeError: expected type int32_t, got 2147483648 - expected PASS
28291 11:05:06 INFO - 2067 INFO TEST-UNEXPECTED-FAIL | toolkit/components/osfile/tests/mochi/test_osfile_back.xul | Test timed out. - expected PASS
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dteller)
Apparently, GENERIC_READ is indeed supposed to be a DWORD, so a signed int. So INT_CONSTANT seems to be the right way to define it.
Flags: needinfo?(dteller)
DWORD is unsigned.
You need to log in before you can comment on or make changes to this bug.