Closed
Bug 1072922
Opened 10 years ago
Closed 9 years ago
fcntl constants and flock strutured to OS.File.Constantcs.libc
Categories
(Toolkit Graveyard :: OS.File, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla36
People
(Reporter: noitidart, Assigned: harshil158, Mentored)
Details
(Whiteboard: [lang=c++][good first bug])
Attachments
(1 file, 6 obsolete files)
2.46 KB,
patch
|
Yoric
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:33.0) Gecko/20100101 Firefox/33.0 Build ID: 20140922173023 Steps to reproduce: Created fcntl in js-ctypes: https://gist.github.com/Noitidart/f05ffde7dcaffe792a4d Actual results: Found that constants vary per OS Expected results: so was recommended to add constants to libc.
Updated•10 years ago
|
Mentor: dteller
Updated•10 years ago
|
Whiteboard: [lang=c++][good first bug]
Comment 2•10 years ago
|
||
noitidart: You might want to read this https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch I'm not a developer but i will try to help if you have questions
hello sir, i am interested in contributing to this bug.I am very much new here....can you guide me getting started with firefox and with this bug?
Comment 4•10 years ago
|
||
(In reply to noitidart from comment #1) > patch ready: > > https://gist.github.com/Noitidart/2c53244c2d65acec518a Could you try creating an actual patch from this? There's instructions here: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I... on how to create a patch file based on your changes to the source dir.
Flags: needinfo?(noitidart)
Ill try thanks. I got some lab work to do today, lab tomorrow from 430-930. If pranali wants to take this im cool. I have another bug im thinking of filing and contributing on.
Flags: needinfo?(noitidart)
Assignee | ||
Comment 6•10 years ago
|
||
Can someone explain me what we have to do in this bug and tell me if anybody has been assigned or not. If not then it would be grateful that somebody guide me into fixing it as this would be my first bug. Thanks
Hi Harshil. I don't think anyone was assign, I was trying to do this but I got real busy with helping others with add-ons and trying to get my own add-ons supported for e10 etc and then school and work. Basically this brings support for flock. See the gist above thats basically all that needs to go in.
Assignee | ||
Comment 8•10 years ago
|
||
I would like to work on the code, how to get is assigned?
Assignee | ||
Comment 9•10 years ago
|
||
Assignee | ||
Comment 10•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8524063 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → harshil158
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Updated•10 years ago
|
Attachment #8524095 -
Flags: review?(dteller)
Comment on attachment 8524095 [details] [diff] [review] "Add libc constants for flock structure type." Review of attachment 8524095 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me, if it compiles on all platforms. Have you already run this through the Try server?
Attachment #8524095 -
Flags: review?(dteller) → review+
Oh, I forgot: could you change your commit message to add ",r=Yoric" at the end of the title?
Assignee | ||
Comment 13•10 years ago
|
||
Yes i will add that, and I have only ran it on my machine and it was working fine.(the offset values were as expected)
Assignee | ||
Comment 14•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8524095 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8524678 -
Flags: review?(dteller)
Comment on attachment 8524678 [details] [diff] [review] Add libc constants for flock structure type, Review of attachment 8524678 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Let's see how it works on the Try Server: https://hg.mozilla.org/try/pushloghtml?changeset=84f768a4657c
Attachment #8524678 -
Flags: review?(dteller) → review+
Sorry, that was meant to be https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=eacc77b3aa9a As you can see, it won't build under Windows :/
Assignee | ||
Comment 17•10 years ago
|
||
Umm, what to do then?
I believe that your changes don't make sense under Windows, so they should be wrapped in a `#if defined(XP_UNIX) / #endif`.
Reporter | ||
Comment 19•10 years ago
|
||
Yeah this is only for mac and linux'es. what does XP in the XP_UNIX stand for? i alwasy thought it mean winXP or Unix
XP = Cross Platform. That's older than Windows XP.
Assignee | ||
Comment 21•10 years ago
|
||
Okay, but wont it work only if there was no problem in windows in first place.
Reporter | ||
Comment 22•10 years ago
|
||
Oh cool thanks yoric man!
Assignee | ||
Comment 23•10 years ago
|
||
Okay, but wont that work only if there was no problem in windows in first place.
There was no problem in Windows. This is a new feature that can work only under Unix.
Assignee | ||
Comment 25•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8524678 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8526231 -
Flags: review?(dteller)
Assignee | ||
Comment 26•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8526231 -
Attachment is obsolete: true
Attachment #8526231 -
Flags: review?(dteller)
Assignee | ||
Updated•10 years ago
|
Attachment #8526241 -
Flags: review?(dteller)
Comment on attachment 8526241 [details] [diff] [review] Add libc constants for flock structure type, Review of attachment 8526241 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/OSFileConstants.cpp @@ +480,5 @@ > INT_CONSTANT(SEEK_END), > INT_CONSTANT(SEEK_SET), > > + // fcntl command values > + #if defined(XP_UNIX) No space before #if or #endif. @@ +481,5 @@ > INT_CONSTANT(SEEK_SET), > > + // fcntl command values > + #if defined(XP_UNIX) > + INT_CONSTANT(F_GETLK), Please keep the indentation of the rest of the file. @@ +489,5 @@ > + // flock type values > + INT_CONSTANT(F_RDLCK), > + INT_CONSTANT(F_WRLCK), > + INT_CONSTANT(F_UNLCK), > + #endif No whitespace at the end of the line. @@ +597,5 @@ > // Size > { "OSFILE_SIZEOF_DIRENT", INT_TO_JSVAL(sizeof (dirent)) }, > > + // Defining |flock|. > + #if defined(XP_UNIX) Same remarks as above.
Attachment #8526241 -
Flags: review?(dteller) → feedback+
Assignee | ||
Comment 28•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8526241 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8526263 -
Flags: review?(dteller)
Comment on attachment 8526263 [details] [diff] [review] Add libc constants for flock structure type, Review of attachment 8526263 [details] [diff] [review]: ----------------------------------------------------------------- Almost there. ::: dom/system/OSFileConstants.cpp @@ +489,5 @@ > + // flock type values > + INT_CONSTANT(F_RDLCK), > + INT_CONSTANT(F_WRLCK), > + INT_CONSTANT(F_UNLCK), > +#endif Nit: No whitespace at the end of the line. On the other hand, if you look at the rest of the file, this should be #endif // defined(XP_UNIX) @@ +604,5 @@ > + { "OSFILE_OFFSETOF_FLOCK_L_LEN", INT_TO_JSVAL(offsetof (struct flock, l_len)) }, > + { "OSFILE_OFFSETOF_FLOCK_L_PID", INT_TO_JSVAL(offsetof (struct flock, l_pid)) }, > + { "OSFILE_OFFSETOF_FLOCK_L_TYPE", INT_TO_JSVAL(offsetof (struct flock, l_type)) }, > + { "OSFILE_OFFSETOF_FLOCK_L_WHENCE", INT_TO_JSVAL(offsetof (struct flock, l_whence)) }, > +#endif Same here.
Attachment #8526263 -
Flags: review?(dteller) → feedback+
Assignee | ||
Comment 30•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8526263 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8526476 -
Flags: review?(dteller)
Comment on attachment 8526476 [details] [diff] [review] Add libc constants for flock structure type, Review of attachment 8526476 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Let's see if it works as expected: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6d90e1a71794
Attachment #8526476 -
Flags: review?(dteller) → review+
Tests look good. Here we go.
Keywords: checkin-needed
Reporter | ||
Comment 33•9 years ago
|
||
What does `#if defined(XP_UNIX)` do if it encounter Mac OS X? Because this is also for Mac.
Comment 34•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/a7bd9b15a071
Keywords: checkin-needed
Whiteboard: [lang=c++][good first bug] → [lang=c++][good first bug][fixed-in-fx-team]
Comment 35•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a7bd9b15a071
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [lang=c++][good first bug][fixed-in-fx-team] → [lang=c++][good first bug]
Target Milestone: --- → mozilla36
Updated•10 months ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•