-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Working on Windows #10
base: master
Are you sure you want to change the base?
Conversation
@@ -2,6 +2,8 @@ | |||
|
|||
Working `mmap()` binding for node.js | |||
|
|||
This version taken from https://github.com/indutny/mmap.js and modified works also under Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess we should remove these lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes of course!
src/win/getpagesize.h
Outdated
@@ -0,0 +1,34 @@ | |||
/* getpagesize emulation for systems where it cannot be done in a C macro. | |||
|
|||
Copyright (C) 2007, 2009-2012 Free Software Foundation, Inc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't include GPL code in an MIT licensed codebase, is this the only file that contains GPL licensed code or is there more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only two files taken from the web and edited are getpagesize.h
and mman.h
. The former has the license specified at the beginning of the file; the latter no and so I cannot guess which type of license it has. Some similar versions can be found: https://github.com/dimichgh/mmap.js/blob/master/src/winmmap.h , https://github.com/ozra/mmap-io/blob/master/src/mman.h but always without license.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid GPL code is not allowed in MIT licensed project (such as this). Sorry!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking for alternative implementations but being ony 4 lines of code the solution is always the same. Also on stackoverflow (https://stackoverflow.com/a/7850494) is done in the same way using different variable names. In fact it is only a call to a Windows system function as explained in the official documentation: https://msdn.microsoft.com/en-us/library/windows/desktop/ms724958%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396.
How can I rewrite and include it without violating licenses?
Thanks a lot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to remove GPL code in the best possible way. Can you check if now it's ok here https://github.com/lucari93/mmap.js? Thanks
src/win/mman.h
Outdated
|
||
inline void* mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) { | ||
if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC)) { | ||
printf("caso0\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to remove some printf
. They are here only for debugging.
I don't know if you have already received a notification or not, I've tried to remove GPL code. Let me know if now it is ok. Thanks. |
No description provided.