Mappings for methods related to process creation. (Unix) #1421
Conversation
There was a problem hiding this comment.
Do not merge yet, still WIP.
There's a feature to make a PR a draft, which sends this same message. I've toggled that for you.
See reviews inline, dealing mostly with the documentation.
My biggest concern is seeing the GNU documentation on functions that you are putting in the Unix LibC API. I have not researched all of them but you need to be sure they are standard across Unix flavors. Being POSIX-standard is probably a good first step.
You will need to write test cases to exercise all this code and show that it works (including on Linux and Unix).
- resolved other docs issues
| * to be noticed but the child whose PID was specified is not one of them, waitpid will block or return zero as described above. | ||
| * @see <a href="https://www.freebsd.org/cgi/man.cgi?query=waitpid">waitpid(3)</a> | ||
| */ | ||
| int waitpid(int pid, int status, int options); |
There was a problem hiding this comment.
Changed statusPtr (type Object) to status (type int) because that's the actual data type. But I'm not sure if JNA actually passes over the pointer of the java status int and lets its value be changeable in C.
There was a problem hiding this comment.
status should be IntByReference with the int * type.
| else if (pid < 0) // Fork failed | ||
| status = -1; | ||
| else { // This is the parent process. Wait for the child to complete. | ||
| if (LibC.INSTANCE.waitpid(pid, status, 0) != pid) |
There was a problem hiding this comment.
This function must be tested before, before using it in this test.
Gotta find a way of testing it.
There was a problem hiding this comment.
You could share memory between the two processes and write to it.
| */ | ||
| int fexecve(int fd, String[] argv, String[] env); | ||
|
|
||
| int WAIT_ANY = -1; |
There was a problem hiding this comment.
constants should go at the top of the file
| * {@code close()} should not be retried after an error. | ||
| */ | ||
| int close(int fd); | ||
|
|
There was a problem hiding this comment.
Still a big chunk of changes not associated with your PR. It's ok for a few small ones to slip in but do your best to leave existing code alone.
| */ | ||
| int munmap(Pointer addr, size_t length); | ||
|
|
||
|
|
||
| // Processes: |
There was a problem hiding this comment.
A preference here, if you're going to add a standalone comment introducing the rest, at least have it contain a bit more... like "mappings for spawning new processes" or somesuch. And I'd like the slashdot style comment for these but that's also purely a preference.
| * to be noticed but the child whose PID was specified is not one of them, waitpid will block or return zero as described above. | ||
| * @see <a href="https://www.freebsd.org/cgi/man.cgi?query=waitpid">waitpid(3)</a> | ||
| */ | ||
| int waitpid(int pid, int status, int options); |
There was a problem hiding this comment.
status should be IntByReference with the int * type.
| * the received status information. | ||
| * @see <a href="https://www.freebsd.org/cgi/man.cgi?query=waitpid">wait(1)</a> | ||
| */ | ||
| int wait(int status); |
There was a problem hiding this comment.
Similar to above, the arg is IntByReference.
| * {@link ErrNo#EMFILE}, {@link ErrNo#ENFILE}, {@link ErrNo#ENOENT}, {@link ErrNo#ENOSPC}, | ||
| * {@link ErrNo#ENXIO}, {@link ErrNo#EROFS}. | ||
| */ | ||
| int open(String filename, int[] flags); // TODO what is mean with: "int flags[, mode_t mode]" https://www.gnu.org/software/libc/manual/html_mono/libc.html#Opening-and-Closing-Files |
There was a problem hiding this comment.
The square brackets typically denote optional arguments. This corresponds to the two overloaded methods at the man page for open (https://man7.org/linux/man-pages/man2/open.2.html), you have int open(const char *pathname, int flags); and int open(const char *pathname, int flags, mode_t mode);. So this method should just have an int for the flags; if you want to map the other one it'd be a separate mapping.
| * @param mode | ||
| * @return | ||
| */ | ||
| int creat(String filename, mode_t mode); // TODO how to handle mode_t type in Java? |
There was a problem hiding this comment.
The javadoc says this function is obsolete, so you probably shouldn't map it. That means you won't have to figure out how to map mode_t. :-)
It's a variable length type, either short or int but I think a relic from the days of using 32-bit variables on 16-bit operating systems. The "proper" way to handle that would be to have the native mappings get a sizeof(mode_t) and save it in a varable you can access like we do for size_t. In reality you could probably map it to an int since nobody's on a 16-bit OS anymore.
Except it's obsolete so just delete this mapping.
| * @return 0 on success or -1 on failure ({@code close()} should not be retried after an error). The following errno error conditions are defined for this function: | ||
| * {@link ErrNo#EBADF}, {@link ErrNo#EINTR}, {@link ErrNo#ENOSPC}, {@link ErrNo#EIO}, {@link ErrNo#EDQUOT}. | ||
| */ | ||
| int close(int fd); |
There was a problem hiding this comment.
This got moved from up above, right? What are the changes?
| else if (pid < 0) // Fork failed | ||
| status = -1; | ||
| else { // This is the parent process. Wait for the child to complete. | ||
| if (LibC.INSTANCE.waitpid(pid, status, 0) != pid) |
There was a problem hiding this comment.
You could share memory between the two processes and write to it.
|
This is WIP for more than a year, closing for now. Feel free to reopen if you want to continue work. |
|
Oh sorry totally forgot about it, since I stopped needing it for my project. |
Related to #1417.
Do not merge yet, still WIP.
Since I am a noob at native libs and JNA I already created this PR to get some feedback and make sure everything I did so far is right, before mapping the rest of the process-related methods.