-
Notifications
You must be signed in to change notification settings - Fork 39
Fantastic js-ctypes! select and fd_set #277
Comments
PS I saw the note "this should hopefully work" which kind of worries me:
I'm not intending to use this on mac but it would be awesome if we can improve this to not have any assumptions. Can you please mention to me in what direction I can go to make these not based on any assumptions, like I can calculate if the system is 64bit or not with no assumption with:
|
Hi Noitidart, Unfortunately I'm very busy the next couple of days, so I can't help
That looks like it will run on most Unix systems based on my reading of since all of those systems put the array at the beginning of the fd_set However, it's not guaranteed to work according to the POSIX standard, It definitely won't work on Windows, as you can see from the layout However, it should be relatively easy to get around that (just add the |
Thank you very much @zentner-kyle for that input! I'll definitely use that and share any improvements we make. As of right I dropped your code into mine and it's working so thank you very much for that! :) I noticed I had to do fd_set every time in the loop. Also would it be safe to cache the value of |
Hey @zentner-kyle we have I wanted to rewrite this so it works perfectly but I was wondering if you had time :p I was in middle of some open source stuff but after I finish that Ill definitely revisit this if you haven't knocked it out by then :) A sincere thanks again for sharing this awesomeness in the open! |
Hi @zentner-kyle I'm wrapping up my file watcher, I converted it to a submodule so it can be included into any addon, and possibly drop it into Firefox itself - https://bugzilla.mozilla.org/show_bug.cgi?id=958280 I was wondering your thoughts on this
And to detect
And then for
|
Hi @Noitidart, Sorry for the slow response. I kinda get a lot of Github notifications, and this one was lost in the flood. I think the only issue you need to address is that fd_set is assumed by native code to have a specific size. If you allocate an array smaller than that, and pass it to select as an fd_set, you can segfault the process you're running in. Unfortunately, the size varies between each Unix. Fortunately, being too large should not be a problem. As far as I can tell, the largest fd_set variant is that used by Linux, which is 1024 bits. This variant is used by almost every Unix, excluding at least NetBSD and Minix (both of which use a smaller size). So if you always make setSize 128 or greater, we can avoid crashing. Unforunately, there's go guarantee that wordSize is correct in general. From reading the headers, it is correct for x86 Linux and x86_64 Linux. It is probably also correct for Linux on other platforms (like ARM), but I'm not positive. However, it appears that OSX and several of the BSD's always uses 32 bit integers for each entry in fd_set. Fortunately, if this value is wrong, the result only changes on big endian architectures, and most processors are little endian these days. Besides that, it looks like there might be a bug in the clear_bit function, since it doesn't shift by the index. I think the correct function would be: buffer.clear_bit = function(index) {
buffer[make_index(index)] &= ~(1 << (index % 8));
}; If you'd like to point me at a repo to review instead, I'd be happy to take a look at that. |
Thanks very much @zentner-kyle - I really appreciate that background and checking. Especially the platform dependent explanation. The repo is here - https://github.com/Noitidart/jscFileWatcher/ It is meant for use from a ChromeWorker instance, here is live demo usage - https://github.com/Noitidart/CommPlayground/tree/jscfilewatcher-demo The non-setup code is here - https://github.com/Noitidart/CommPlayground/blob/jscfilewatcher-demo/resources/scripts/MainWorker.js There is some setup on the mainthread ( https://github.com/Noitidart/CommPlayground/blob/jscfilewatcher-demo/bootstrap.js ) as Gio file watcher requires callback on mainthread (from what I could gather). Right now (master commit of jscFileWatcher module) I'm forcing it to use inotify on all linux as i'm trying to fix the poll there. I really badly wanted to use For inotify, I'm getting an unexplained EINTR ( https://bugzilla.mozilla.org/show_bug.cgi?id=1288293 ), but everything is almost set. I just have to fix this EINTR issue, and then make the delivery to the callback set up the by the developer in a common way across all platforms. Right now you will see it successfully trigger callback on all Windows, OSX (10.7), GTK (if inotify is not forced) systems. I really appreciate your input! |
I was working on a bugzilla issue to bring a file watching API and needed to use
select
and needed theFD_SET
macro. (Noitidart/jscFileWatcher#12) I found you guys already wrote up this macro for use withselect
here:tenshi/angel-player/src/chrome/content/common/serport_posix.js
Line 497 in 9b32732
This is real awesome thank you! May I "borrow" this please haha
The text was updated successfully, but these errors were encountered: