apache / nuttx

Apache NuttX is a mature, real-time embedded operating system (RTOS)
https://nuttx.apache.org/
Apache License 2.0
2.6k stars 1.11k forks source link

Nuttx file descriptor dup operation issue #6710

Open Donny9 opened 2 years ago

Donny9 commented 2 years ago

Issue

The dup operation of nuttx is different from that of freebsd and opengroup.

/* A file descriptor is an index into an array of such types. The type is used to

struct filenode { int fn_oflags; / Open or duplicate mode flags / FAR struct file fn_file; / The pointer representation of an open file */ };

/* This defines a two layer array of files indexed by the file descriptor.

struct filelist { sem_t fl_sem; / Manage access to the file list / uint8_t fl_rows; / The number of rows of fl_files array / FAR struct filenode *fl_files; / The pointer of two layer file descriptors array */ };


And it also needs to modify file_openation:dup, directly delete or remove unnecessary features.

# Refer to
https://www.freebsd.org/cgi/man.cgi?query=dup&sektion=2
https://pubs.opengroup.org/onlinepubs/009604499/functions/dup.html
xiaoxiang781216 commented 2 years ago

@pkarashchenko @patacongo @acassis @masayuki2009 what do you think about this bug?

pkarashchenko commented 2 years ago

I see that FreeBSD indeed includes such description, but I failed to find any similar lines in https://pubs.opengroup.org/onlinepubs/9699919799/functions/dup.html

Additionally I can find in description from OpenGroup: The dup() function provides an alternative interface to the service provided by fcntl() using the F_DUPFD command. The call dup(fildes) shall be equivalent to:

fcntl(fildes, F_DUPFD, 0);

But fail to find similar in description from FreeBSD

Donny9 commented 1 year ago

@pkarashchenko

I see that FreeBSD indeed includes such description, but I failed to find any similar lines in https://pubs.opengroup.org/onlinepubs/9699919799/functions/dup.html

Additionally I can find in description from OpenGroup: The dup() function provides an alternative interface to the service provided by fcntl() using the F_DUPFD command. The call dup(fildes) shall be equivalent to:

fcntl(fildes, F_DUPFD, 0);

But fail to find similar in description from FreeBSD

@pkarashchenko FreeBsd also have similar behavior in desctiption: https://man.freebsd.org/cgi/man.cgi?query=dup2&apropos=0&sektion=2&manpath=FreeBSD+9.0-RELEASE&arch=amd64&format=html#:~:text=The%20object%20referenced,between%20the%20references.

The object referenced by the descriptor does not distinguish between oldd
and newd in any way.  Thus if newd and oldd are duplicate references to
an open file, [read(2)],  [write(2)] and [lseek(2)] calls all move a single
pointer into the file, and append mode, non-blocking I/O and asynchronous
I/O options are shared between the references.

So any good suggestions to solve this problem? @pkarashchenko @acassis If there are no good suggestions, I will use filenode to solve this problem in the near future

pkarashchenko commented 1 year ago

@Donny9 It's a bit find out from a description what is the issue from POSIX perspective, but finally I think I managed to understand what you are talking about. It is https://pubs.opengroup.org/onlinepubs/9699919799/functions/dup.html

The dup2() function shall cause the file descriptor fildes2 to refer to the same open file description as the file descriptor
fildes and to share any locks, and shall return fildes2.

Is that correct? If yes, then it depends on the interpretation if refer to the same open file description is equivalent to does not distinguish between oldd and newd in any way.

If I still didn't understood the issue correctly then please explain it in more clear manner like:

Donny9 commented 11 months ago

@pkarashchenko This examples can run with pass result on linux, but can't pass on nuttx, because this issue: newd and oldd can't share file offset, because they are different file handle in nuttx.

//#include <nuttx/config.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
#include <syslog.h>
//#include "test.h"
#define TESTFILE "testDup2File1"
#define TEST_FAIL -1
#define TEST_PASS 0

/****************************************************************************
 * Name: dup
 * Example description:
    1. Open a file and get the file identifier fd.
    2. Copy the file identifier to newfd
    3. Open the file through newfd. And check the return results
 * Test item: open()  write() dup2()
 * Expect results: TEST PASSED
 ****************************************************************************/

int main(void)
{
    char buf[16] = {0};

    off_t currpos;
    int fd1 = open(TESTFILE, O_RDWR | O_CREAT, 0777);
    if (fd1 == -1)
    {
        printf("open file fail !\n");
        return TEST_FAIL;
    }

    int fd2 = open(TESTFILE, O_RDWR | O_CREAT, 0777);
    if (fd2 == -1)
    {
        close(fd1);
        printf("open file fail !\n");
        return TEST_FAIL;
    }

    int ret = dup2(fd1, fd2);
    if (ret == -1)
    {
        close(fd1);
        close(fd2);
        printf("dup2 error ,  return : %d!\n", ret);
        return TEST_FAIL;
    }
    syslog(LOG_INFO, "current fd = %d\n", ret);
    char *buf1 = "hello ";
    char *buf2 = "world!";
    write(fd1, buf1, strlen(buf1));
    write(fd2, buf2, strlen(buf2));

    fsync(fd1);

    /* reset file pos use fd2 */
    lseek(fd2, 0, SEEK_SET);

    /* Check if file pos is shared */
    currpos = lseek(fd1, 0, SEEK_CUR);
    if (currpos != 0)
    {
        printf("Do not shared file pos after dup2()\n");
        close(fd1);
        close(fd2);
        return TEST_FAIL;
    }

    ret = read(fd1, buf, 16);
    if (ret <= 0)
    {
        printf("read fail\n");
        close(fd1);
        close(fd2);
        return TEST_FAIL;
    }
    close(fd1);
    close(fd2);

    if (strncmp(buf, "hello world!", 12) == 0)
    {
        printf("test pass\n");
        return TEST_PASS;
    }

    printf("test fail\n");
    return TEST_FAIL;
}
pkarashchenko commented 11 months ago

@Donny9 what about other OSes? I will try to run it on MAC OS and come back with results

pkarashchenko commented 11 months ago

The MACOS works the same and test pass. On your proposal I do not fully understand why struct filenode is needed. Could you please explain more. I see that struct filenode contains fn_oflags and struct file contains f_oflags, so what is the difference between those two?

Donny9 commented 11 months ago

xplain more. I see that struct filenode contains fn_oflags and struct file contains f_oflags, so what is the difference between those two?

oldd and newd need to share file offsets and other information, but they need to have their own flags. We can remove the flags in the file structure, but it will change a lot. Therefore, we need to make oldd and newd point to the same file structure, so let the file be one level below and use filenode as the file opening context.

pkarashchenko commented 11 months ago

But leaving flags in both file and filenode structures will lead to a duplication? Or some flags will stay in file while other will reside in filenode? BTW I think that there is a typo in your test example. I didn't try to run it on NuttX, but believe that line ret = read(fd1, buf, 16); should be ret = read(ret, buf, 16); otherwise I do not see where dup2 result is used