| Message ID | 20220111145817.22170-18-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mini-os: remove struct file dependency on config | expand |
On 11/01/2022 14:58, Juergen Gross wrote: > Introduce file type specific function vectors for all the needed file > operations which are file type specific in order to prepare replacing > the large switch statements in each generic file function. > > Add a function to allocate a new file type dynamically in order to > prepare removing direct dependencies to external components in the > future. For this reason switch the file type from an enum to an > unsigned int. Prepare removal of some statically defines file types > by putting them at the end of the defined list. > > Change the debug output for the file type from a single character to > a string in order to support a future removal of the file_types[] > array. > > Signed-off-by: Juergen Gross <jgross@suse.com> A few misc observations. > diff --git a/include/lib.h b/include/lib.h > index 7a0546b..b40e213 100644 > --- a/include/lib.h > +++ b/include/lib.h > @@ -154,25 +155,51 @@ do { \ > void sanity_check(void); > > #ifdef HAVE_LIBC > -enum fd_type { > - FTYPE_NONE = 0, > - FTYPE_CONSOLE, > - FTYPE_FILE, > - FTYPE_XENBUS, > - FTYPE_XC, > - FTYPE_EVTCHN, > - FTYPE_GNTMAP, > - FTYPE_SOCKET, > - FTYPE_TAP, > - FTYPE_BLK, > - FTYPE_KBD, > - FTYPE_FB, > - FTYPE_MEM, > - FTYPE_SAVEFILE, > - FTYPE_TPMFRONT, > - FTYPE_TPM_TIS, > +#define FTYPE_NONE 0 > +#define FTYPE_CONSOLE 1 > +#define FTYPE_FILE 2 > +#define FTYPE_SOCKET 3 > +#define FTYPE_MEM 4 > +#define FTYPE_SAVEFILE 5 > +#define FTYPE_FB 6 > +#define FTYPE_KBD 7 > +#define FTYPE_TAP 8 > +#define FTYPE_BLK 9 > +#define FTYPE_TPMFRONT 10 > +#define FTYPE_TPM_TIS 11 > +#define FTYPE_XENBUS 12 > +#define FTYPE_GNTMAP 13 > +#define FTYPE_EVTCHN 14 > +#define FTYPE_XC 15 > +#define FTYPE_N 16 > +#define FTYPE_SPARE 16 > + > +typedef int file_read_func(int fd, void *buf, size_t nbytes); > +typedef int file_write_func(int fd, const void *buf, size_t nbytes); > +typedef off_t file_lseek_func(int fd, off_t offset, int whence); > +typedef int file_close_func(int fd); > +typedef int file_fstat_func(int fd, struct stat *buf); > +typedef int file_fcntl_func(int fd, int cmd, va_list args); > +typedef bool file_select_func(int fd); Are these typedef's useful? I don't see them used anywhere other than the file_ops structure. > + > +struct file_ops { > + const char *name; > + file_read_func *read; > + file_write_func *write; > + file_lseek_func *lseek; > + file_close_func *close; > + file_fstat_func *fstat; > + file_fcntl_func *fcntl; > + file_select_func *select_rd; > + file_select_func *select_wr; > }; > > +unsigned int alloc_file_type(struct file_ops *ops); const. > + > +off_t lseek_default(int fd, off_t offset, int whence); > +bool select_yes(int fd); > +bool select_read_flag(int fd); > + > LIST_HEAD(evtchn_port_list, evtchn_port_info); > > struct evtchn_port_info { > @@ -183,7 +210,7 @@ struct evtchn_port_info { > }; > > struct file { > - enum fd_type type; > + unsigned int type; > bool read; /* maybe available for read */ > off_t offset; > union { > @@ -199,7 +226,7 @@ struct file { > extern struct file files[]; > > struct file *get_file_from_fd(int fd); > -int alloc_fd(enum fd_type type); > +int alloc_fd(unsigned int type); > void close_all_files(void); > extern struct thread *main_thread; > void sparse(unsigned long data, size_t size); > diff --git a/lib/sys.c b/lib/sys.c > index 323a7cd..c327247 100644 > --- a/lib/sys.c > +++ b/lib/sys.c > @@ -98,6 +98,39 @@ struct file files[NOFILE] = { > { .type = FTYPE_CONSOLE }, /* stderr */ > }; > > +static struct file_ops file_ops_none = { > + .name = "none", > +}; > + > +static struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = { > + [FTYPE_NONE] = &file_ops_none, > +}; Both want to be const, because all file_ops ought to live in rodata. > + > +unsigned int alloc_file_type(struct file_ops *ops) > +{ > + unsigned int i; > + > + pthread_mutex_lock(&fd_lock); > + > + for ( i = FTYPE_N; i < ARRAY_SIZE(file_ops) && file_ops[i]; i++ ); No need for a for loop. Given the mutex, you can safely do something like this: static unsigned int i = FTYPE_N; BUG_ON(i == ARRAY_SIZE(file_ops)); file_ops[i++] = ops; Alternatively, if you do want to keep the loop, we might want to confirm that the same ops structure doesn't get registered twice. > + BUG_ON(i == ARRAY_SIZE(file_ops)); > + file_ops[i] = ops; > + > + pthread_mutex_unlock(&fd_lock); > + > + printk("New file type \"%s\"(%u) allocated\n", ops->name, i); > + > + return i; > +} > + > +static struct file_ops *get_file_ops(unsigned int type) > +{ > + if ( type >= ARRAY_SIZE(file_ops) || !file_ops[type] ) > + return &file_ops_none; > + > + return file_ops[type]; > +} > + > struct file *get_file_from_fd(int fd) > { > if ( fd < 0 || fd >= NOFILE ) > @@ -108,7 +141,7 @@ struct file *get_file_from_fd(int fd) > > DECLARE_WAIT_QUEUE_HEAD(event_queue); > > -int alloc_fd(enum fd_type type) > +int alloc_fd(unsigned int type) > { > int i; > pthread_mutex_lock(&fd_lock); > @@ -249,6 +282,11 @@ int isatty(int fd) > > int read(int fd, void *buf, size_t nbytes) > { > + struct file_ops *ops = get_file_ops(files[fd].type); > + > + if ( ops->read ) > + return ops->read(fd, buf, nbytes); > + > switch (files[fd].type) { > case FTYPE_SAVEFILE: > case FTYPE_CONSOLE: { > @@ -328,6 +366,11 @@ int read(int fd, void *buf, size_t nbytes) > > int write(int fd, const void *buf, size_t nbytes) > { > + struct file_ops *ops = get_file_ops(files[fd].type); > + > + if ( ops->write ) > + return ops->write(fd, buf, nbytes); > + > switch (files[fd].type) { > case FTYPE_SAVEFILE: { > int ret = 0, tot = nbytes; > @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes) > return -1; > } > > +off_t lseek_default(int fd, off_t offset, int whence) > +{ > + switch ( whence ) > + { > + case SEEK_SET: > + files[fd].offset = offset; > + break; > + > + case SEEK_CUR: > + files[fd].offset += offset; > + break; > + > + case SEEK_END: > + { > + struct stat st; > + int ret; > + > + ret = fstat(fd, &st); > + if ( ret ) > + return -1; > + files[fd].offset = st.st_size + offset; > + break; > + } > + > + default: > + errno = EINVAL; > + return -1; > + } > + > + return files[fd].offset; > +} > + > off_t lseek(int fd, off_t offset, int whence) > { > + struct file_ops *ops = get_file_ops(files[fd].type); > + > + if ( ops->lseek ) > + return ops->lseek(fd, offset, whence); > + > switch(files[fd].type) { > #ifdef CONFIG_BLKFRONT > case FTYPE_BLK: > @@ -393,28 +473,7 @@ off_t lseek(int fd, off_t offset, int whence) > return (off_t) -1; > } > > - switch (whence) { > - case SEEK_SET: > - files[fd].offset = offset; > - break; > - case SEEK_CUR: > - files[fd].offset += offset; > - break; > - case SEEK_END: > - { > - struct stat st; > - int ret; > - ret = fstat(fd, &st); > - if (ret) > - return -1; > - files[fd].offset = st.st_size + offset; > - break; > - } > - default: > - errno = EINVAL; > - return -1; > - } > - return files[fd].offset; > + return lseek_default(fd, offset, whence); > } > > int fsync(int fd) { > @@ -425,8 +484,15 @@ int fsync(int fd) { > int close(int fd) > { > int res = 0; > + struct file_ops *ops = get_file_ops(files[fd].type); > > printk("close(%d)\n", fd); > + if ( ops->close ) > + { > + res = ops->close(fd); > + goto out; > + } > + > switch (files[fd].type) { > default: > break; > @@ -497,6 +563,7 @@ int close(int fd) > return -1; > } > > + out: > memset(files + fd, 0, sizeof(struct file)); > files[fd].type = FTYPE_NONE; > return res; > @@ -521,7 +588,13 @@ int stat(const char *path, struct stat *buf) > > int fstat(int fd, struct stat *buf) > { > + struct file_ops *ops = get_file_ops(files[fd].type); > + > init_stat(buf); > + > + if ( ops->fstat ) > + return ops->fstat(fd, buf); > + > switch (files[fd].type) { > case FTYPE_SAVEFILE: > case FTYPE_CONSOLE: > @@ -587,6 +660,18 @@ int fcntl(int fd, int cmd, ...) > { > long arg; > va_list ap; > + int res; > + struct file_ops *ops = get_file_ops(files[fd].type); > + > + if ( ops->fcntl ) > + { > + va_start(ap, cmd); > + res = ops->fcntl(fd, cmd, ap); > + va_end(ap); > + > + return res; > + } > + > va_start(ap, cmd); > arg = va_arg(ap, long); > va_end(ap); > @@ -640,18 +725,29 @@ int closedir(DIR *dir) > /* We assume that only the main thread calls select(). */ > > #if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE) > -static const char file_types[] = { > - [FTYPE_NONE] = 'N', > - [FTYPE_CONSOLE] = 'C', > - [FTYPE_XENBUS] = 'S', > - [FTYPE_XC] = 'X', > - [FTYPE_EVTCHN] = 'E', > - [FTYPE_SOCKET] = 's', > - [FTYPE_TAP] = 'T', > - [FTYPE_BLK] = 'B', > - [FTYPE_KBD] = 'K', > - [FTYPE_FB] = 'G', > +static const char *file_types[] = { static const char *const file_types[] > + [FTYPE_NONE] = "none", > + [FTYPE_CONSOLE] = "console", > + [FTYPE_XENBUS] = "xenbus", > + [FTYPE_XC] = "ctrl", > + [FTYPE_EVTCHN] = "evtchn", > + [FTYPE_SOCKET] = "socket", > + [FTYPE_TAP] = "net", > + [FTYPE_BLK] = "blk", > + [FTYPE_KBD] = "kbd", > + [FTYPE_FB] = "fb", > }; > + > +static char *get_type_name(unsigned int type) const. > +{ > + if ( type < ARRAY_SIZE(file_ops) && file_ops[type] ) > + return file_ops[type]->name; > + > + if ( type < ARRAY_SIZE(file_types) && file_types[type] ) > + return file_types[type]; > + > + return "none"; > +} > #endif > #ifdef LIBC_DEBUG > static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *timeout) > @@ -663,7 +759,7 @@ static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except > if (FD_ISSET(i, set)) { \ > if (comma) \ > printk(", "); \ > - printk("%d(%c)", i, file_types[files[i].type]); \ > + printk("%d(%s)", i, get_type_name(files[i].type)); \ > comma = 1; \ > } \ > } \ > @@ -697,7 +793,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout) > fd = pfd[i].fd; > if (comma) > printk(", "); > - printk("%d(%c)/%02x", fd, file_types[files[fd].type], > + printk("%d(%s)/%02x", fd, get_type_name(files[fd].type), > pfd[i].events); > comma = 1; > } > @@ -709,6 +805,18 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout) > #define dump_pollfds(pfds, nfds, timeout) > #endif > > +bool select_yes(int fd) > +{ > + return true; > +} > + > +bool select_read_flag(int fd) > +{ > + struct file *file = get_file_from_fd(fd); > + > + return file->read; > +} I don't see these getting used, even in a fallback case. > + > /* Just poll without blocking */ > static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds) > { > @@ -762,9 +870,35 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce > for (i = 0; i < nfds; i++) { > switch(files[i].type) { > default: > + { > + struct file_ops *ops = file_ops[files[i].type]; > + > + if ( ops ) > + { > + if ( FD_ISSET(i, readfds) ) > + { > + if ( ops->select_rd && ops->select_rd(i) ) > + n++; > + else > + FD_CLR(i, readfds); > + } > + if ( FD_ISSET(i, writefds) ) > + { > + if ( ops->select_wr && ops->select_wr(i) ) > + n++; > + else > + FD_CLR(i, writefds); > + } > + FD_CLR(i, exceptfds); Hard tab. > + > + break; > + } > + > if (FD_ISSET(i, readfds) || FD_ISSET(i, writefds) || FD_ISSET(i, exceptfds)) > printk("bogus fd %d in select\n", i); > /* Fallthrough. */ > + } Is this fallthrough really appropriate, given the break in the ops case? ~Andrew > + > case FTYPE_CONSOLE: > if (FD_ISSET(i, readfds)) { > if (xencons_ring_avail(files[i].dev)) > @@ -842,7 +976,7 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce > printk("%d(%d): ", nb, sock_n); > for (i = 0; i < nfds; i++) { > if (nbread[i] || nbwrite[i] || nbexcept[i]) > - printk(" %d(%c):", i, file_types[files[i].type]); > + printk(" %d(%c):", i, get_type_name(files[i].type)); > if (nbread[i]) > printk(" %dR", nbread[i]); > if (nbwrite[i])
Andrew Cooper, le mar. 11 janv. 2022 18:08:27 +0000, a ecrit: > On 11/01/2022 14:58, Juergen Gross wrote: > > +typedef int file_read_func(int fd, void *buf, size_t nbytes); > > +typedef int file_write_func(int fd, const void *buf, size_t nbytes); > > +typedef off_t file_lseek_func(int fd, off_t offset, int whence); > > +typedef int file_close_func(int fd); > > +typedef int file_fstat_func(int fd, struct stat *buf); > > +typedef int file_fcntl_func(int fd, int cmd, va_list args); > > +typedef bool file_select_func(int fd); > > Are these typedef's useful? I don't see them used anywhere other than > the file_ops structure. > > > + > > +struct file_ops { > > + const char *name; > > + file_read_func *read; > > + file_write_func *write; > > + file_lseek_func *lseek; > > + file_close_func *close; > > + file_fstat_func *fstat; > > + file_fcntl_func *fcntl; > > + file_select_func *select_rd; > > + file_select_func *select_wr; > > }; I agree, I'd rather see the protoypes inside the struct itself. Samuel
Juergen Gross, le mar. 11 janv. 2022 15:58:16 +0100, a ecrit:
> + for ( i = FTYPE_N; i < ARRAY_SIZE(file_ops) && file_ops[i]; i++ );
For such loops, I'd rather see the ';' on its own line, to make it clear
it's an empty loop.
Samuel
Juergen Gross, le mar. 11 janv. 2022 15:58:16 +0100, a ecrit: > @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes) > return -1; > } > > +off_t lseek_default(int fd, off_t offset, int whence) > +{ > + switch ( whence ) > + { Is there a reason for making this one a separate function, unlike others for which you kept the code in the main function? Apart from that, this looks good to me. > + case SEEK_SET: > + files[fd].offset = offset; > + break; > + > + case SEEK_CUR: > + files[fd].offset += offset; > + break; > + > + case SEEK_END: > + { > + struct stat st; > + int ret; > + > + ret = fstat(fd, &st); > + if ( ret ) > + return -1; > + files[fd].offset = st.st_size + offset; > + break; > + } > + > + default: > + errno = EINVAL; > + return -1; > + } > + > + return files[fd].offset; > +} > + > off_t lseek(int fd, off_t offset, int whence) > { > + struct file_ops *ops = get_file_ops(files[fd].type); > + > + if ( ops->lseek ) > + return ops->lseek(fd, offset, whence); > + > switch(files[fd].type) { > #ifdef CONFIG_BLKFRONT > case FTYPE_BLK: > @@ -393,28 +473,7 @@ off_t lseek(int fd, off_t offset, int whence) > return (off_t) -1; > } > > - switch (whence) { > - case SEEK_SET: > - files[fd].offset = offset; > - break; > - case SEEK_CUR: > - files[fd].offset += offset; > - break; > - case SEEK_END: > - { > - struct stat st; > - int ret; > - ret = fstat(fd, &st); > - if (ret) > - return -1; > - files[fd].offset = st.st_size + offset; > - break; > - } > - default: > - errno = EINVAL; > - return -1; > - } > - return files[fd].offset; > + return lseek_default(fd, offset, whence); > } > > int fsync(int fd) {
Samuel Thibault, le mar. 11 janv. 2022 21:05:47 +0100, a ecrit: > Juergen Gross, le mar. 11 janv. 2022 15:58:16 +0100, a ecrit: > > @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes) > > return -1; > > } > > > > +off_t lseek_default(int fd, off_t offset, int whence) > > +{ > > + switch ( whence ) > > + { > > Is there a reason for making this one a separate function, unlike others > for which you kept the code in the main function? Ah, I guess that it's because in the end, it's the only function that has a default implementation left? And thus the rule is that unless the method is set, the default implementation is ENOSYS? I agree with that, then. Samuel
On 11.01.22 19:08, Andrew Cooper wrote: > On 11/01/2022 14:58, Juergen Gross wrote: >> Introduce file type specific function vectors for all the needed file >> operations which are file type specific in order to prepare replacing >> the large switch statements in each generic file function. >> >> Add a function to allocate a new file type dynamically in order to >> prepare removing direct dependencies to external components in the >> future. For this reason switch the file type from an enum to an >> unsigned int. Prepare removal of some statically defines file types >> by putting them at the end of the defined list. >> >> Change the debug output for the file type from a single character to >> a string in order to support a future removal of the file_types[] >> array. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> > > A few misc observations. > >> diff --git a/include/lib.h b/include/lib.h >> index 7a0546b..b40e213 100644 >> --- a/include/lib.h >> +++ b/include/lib.h >> @@ -154,25 +155,51 @@ do { \ >> void sanity_check(void); >> >> #ifdef HAVE_LIBC >> -enum fd_type { >> - FTYPE_NONE = 0, >> - FTYPE_CONSOLE, >> - FTYPE_FILE, >> - FTYPE_XENBUS, >> - FTYPE_XC, >> - FTYPE_EVTCHN, >> - FTYPE_GNTMAP, >> - FTYPE_SOCKET, >> - FTYPE_TAP, >> - FTYPE_BLK, >> - FTYPE_KBD, >> - FTYPE_FB, >> - FTYPE_MEM, >> - FTYPE_SAVEFILE, >> - FTYPE_TPMFRONT, >> - FTYPE_TPM_TIS, >> +#define FTYPE_NONE 0 >> +#define FTYPE_CONSOLE 1 >> +#define FTYPE_FILE 2 >> +#define FTYPE_SOCKET 3 >> +#define FTYPE_MEM 4 >> +#define FTYPE_SAVEFILE 5 >> +#define FTYPE_FB 6 >> +#define FTYPE_KBD 7 >> +#define FTYPE_TAP 8 >> +#define FTYPE_BLK 9 >> +#define FTYPE_TPMFRONT 10 >> +#define FTYPE_TPM_TIS 11 >> +#define FTYPE_XENBUS 12 >> +#define FTYPE_GNTMAP 13 >> +#define FTYPE_EVTCHN 14 >> +#define FTYPE_XC 15 >> +#define FTYPE_N 16 >> +#define FTYPE_SPARE 16 >> + >> +typedef int file_read_func(int fd, void *buf, size_t nbytes); >> +typedef int file_write_func(int fd, const void *buf, size_t nbytes); >> +typedef off_t file_lseek_func(int fd, off_t offset, int whence); >> +typedef int file_close_func(int fd); >> +typedef int file_fstat_func(int fd, struct stat *buf); >> +typedef int file_fcntl_func(int fd, int cmd, va_list args); >> +typedef bool file_select_func(int fd); > > Are these typedef's useful? I don't see them used anywhere other than > the file_ops structure. I can drop them. > >> + >> +struct file_ops { >> + const char *name; >> + file_read_func *read; >> + file_write_func *write; >> + file_lseek_func *lseek; >> + file_close_func *close; >> + file_fstat_func *fstat; >> + file_fcntl_func *fcntl; >> + file_select_func *select_rd; >> + file_select_func *select_wr; >> }; >> >> +unsigned int alloc_file_type(struct file_ops *ops); > > const. Yes. > >> + >> +off_t lseek_default(int fd, off_t offset, int whence); >> +bool select_yes(int fd); >> +bool select_read_flag(int fd); >> + >> LIST_HEAD(evtchn_port_list, evtchn_port_info); >> >> struct evtchn_port_info { >> @@ -183,7 +210,7 @@ struct evtchn_port_info { >> }; >> >> struct file { >> - enum fd_type type; >> + unsigned int type; >> bool read; /* maybe available for read */ >> off_t offset; >> union { >> @@ -199,7 +226,7 @@ struct file { >> extern struct file files[]; >> >> struct file *get_file_from_fd(int fd); >> -int alloc_fd(enum fd_type type); >> +int alloc_fd(unsigned int type); >> void close_all_files(void); >> extern struct thread *main_thread; >> void sparse(unsigned long data, size_t size); >> diff --git a/lib/sys.c b/lib/sys.c >> index 323a7cd..c327247 100644 >> --- a/lib/sys.c >> +++ b/lib/sys.c >> @@ -98,6 +98,39 @@ struct file files[NOFILE] = { >> { .type = FTYPE_CONSOLE }, /* stderr */ >> }; >> >> +static struct file_ops file_ops_none = { >> + .name = "none", >> +}; >> + >> +static struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = { >> + [FTYPE_NONE] = &file_ops_none, >> +}; > > Both want to be const, because all file_ops ought to live in rodata. file_ops[] can't be const, it will just point to const data. > >> + >> +unsigned int alloc_file_type(struct file_ops *ops) >> +{ >> + unsigned int i; >> + >> + pthread_mutex_lock(&fd_lock); >> + >> + for ( i = FTYPE_N; i < ARRAY_SIZE(file_ops) && file_ops[i]; i++ ); > > No need for a for loop. Given the mutex, you can safely do something > like this: > > static unsigned int i = FTYPE_N; > > BUG_ON(i == ARRAY_SIZE(file_ops)); > file_ops[i++] = ops; Will change it that way. > > Alternatively, if you do want to keep the loop, we might want to confirm > that the same ops structure doesn't get registered twice. > >> + BUG_ON(i == ARRAY_SIZE(file_ops)); >> + file_ops[i] = ops; >> + >> + pthread_mutex_unlock(&fd_lock); >> + >> + printk("New file type \"%s\"(%u) allocated\n", ops->name, i); >> + >> + return i; >> +} >> + >> +static struct file_ops *get_file_ops(unsigned int type) >> +{ >> + if ( type >= ARRAY_SIZE(file_ops) || !file_ops[type] ) >> + return &file_ops_none; >> + >> + return file_ops[type]; >> +} >> + >> struct file *get_file_from_fd(int fd) >> { >> if ( fd < 0 || fd >= NOFILE ) >> @@ -108,7 +141,7 @@ struct file *get_file_from_fd(int fd) >> >> DECLARE_WAIT_QUEUE_HEAD(event_queue); >> >> -int alloc_fd(enum fd_type type) >> +int alloc_fd(unsigned int type) >> { >> int i; >> pthread_mutex_lock(&fd_lock); >> @@ -249,6 +282,11 @@ int isatty(int fd) >> >> int read(int fd, void *buf, size_t nbytes) >> { >> + struct file_ops *ops = get_file_ops(files[fd].type); >> + >> + if ( ops->read ) >> + return ops->read(fd, buf, nbytes); >> + >> switch (files[fd].type) { >> case FTYPE_SAVEFILE: >> case FTYPE_CONSOLE: { >> @@ -328,6 +366,11 @@ int read(int fd, void *buf, size_t nbytes) >> >> int write(int fd, const void *buf, size_t nbytes) >> { >> + struct file_ops *ops = get_file_ops(files[fd].type); >> + >> + if ( ops->write ) >> + return ops->write(fd, buf, nbytes); >> + >> switch (files[fd].type) { >> case FTYPE_SAVEFILE: { >> int ret = 0, tot = nbytes; >> @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes) >> return -1; >> } >> >> +off_t lseek_default(int fd, off_t offset, int whence) >> +{ >> + switch ( whence ) >> + { >> + case SEEK_SET: >> + files[fd].offset = offset; >> + break; >> + >> + case SEEK_CUR: >> + files[fd].offset += offset; >> + break; >> + >> + case SEEK_END: >> + { >> + struct stat st; >> + int ret; >> + >> + ret = fstat(fd, &st); >> + if ( ret ) >> + return -1; >> + files[fd].offset = st.st_size + offset; >> + break; >> + } >> + >> + default: >> + errno = EINVAL; >> + return -1; >> + } >> + >> + return files[fd].offset; >> +} >> + >> off_t lseek(int fd, off_t offset, int whence) >> { >> + struct file_ops *ops = get_file_ops(files[fd].type); >> + >> + if ( ops->lseek ) >> + return ops->lseek(fd, offset, whence); >> + >> switch(files[fd].type) { >> #ifdef CONFIG_BLKFRONT >> case FTYPE_BLK: >> @@ -393,28 +473,7 @@ off_t lseek(int fd, off_t offset, int whence) >> return (off_t) -1; >> } >> >> - switch (whence) { >> - case SEEK_SET: >> - files[fd].offset = offset; >> - break; >> - case SEEK_CUR: >> - files[fd].offset += offset; >> - break; >> - case SEEK_END: >> - { >> - struct stat st; >> - int ret; >> - ret = fstat(fd, &st); >> - if (ret) >> - return -1; >> - files[fd].offset = st.st_size + offset; >> - break; >> - } >> - default: >> - errno = EINVAL; >> - return -1; >> - } >> - return files[fd].offset; >> + return lseek_default(fd, offset, whence); >> } >> >> int fsync(int fd) { >> @@ -425,8 +484,15 @@ int fsync(int fd) { >> int close(int fd) >> { >> int res = 0; >> + struct file_ops *ops = get_file_ops(files[fd].type); >> >> printk("close(%d)\n", fd); >> + if ( ops->close ) >> + { >> + res = ops->close(fd); >> + goto out; >> + } >> + >> switch (files[fd].type) { >> default: >> break; >> @@ -497,6 +563,7 @@ int close(int fd) >> return -1; >> } >> >> + out: >> memset(files + fd, 0, sizeof(struct file)); >> files[fd].type = FTYPE_NONE; >> return res; >> @@ -521,7 +588,13 @@ int stat(const char *path, struct stat *buf) >> >> int fstat(int fd, struct stat *buf) >> { >> + struct file_ops *ops = get_file_ops(files[fd].type); >> + >> init_stat(buf); >> + >> + if ( ops->fstat ) >> + return ops->fstat(fd, buf); >> + >> switch (files[fd].type) { >> case FTYPE_SAVEFILE: >> case FTYPE_CONSOLE: >> @@ -587,6 +660,18 @@ int fcntl(int fd, int cmd, ...) >> { >> long arg; >> va_list ap; >> + int res; >> + struct file_ops *ops = get_file_ops(files[fd].type); >> + >> + if ( ops->fcntl ) >> + { >> + va_start(ap, cmd); >> + res = ops->fcntl(fd, cmd, ap); >> + va_end(ap); >> + >> + return res; >> + } >> + >> va_start(ap, cmd); >> arg = va_arg(ap, long); >> va_end(ap); >> @@ -640,18 +725,29 @@ int closedir(DIR *dir) >> /* We assume that only the main thread calls select(). */ >> >> #if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE) >> -static const char file_types[] = { >> - [FTYPE_NONE] = 'N', >> - [FTYPE_CONSOLE] = 'C', >> - [FTYPE_XENBUS] = 'S', >> - [FTYPE_XC] = 'X', >> - [FTYPE_EVTCHN] = 'E', >> - [FTYPE_SOCKET] = 's', >> - [FTYPE_TAP] = 'T', >> - [FTYPE_BLK] = 'B', >> - [FTYPE_KBD] = 'K', >> - [FTYPE_FB] = 'G', >> +static const char *file_types[] = { > > static const char *const file_types[] Okay. > >> + [FTYPE_NONE] = "none", >> + [FTYPE_CONSOLE] = "console", >> + [FTYPE_XENBUS] = "xenbus", >> + [FTYPE_XC] = "ctrl", >> + [FTYPE_EVTCHN] = "evtchn", >> + [FTYPE_SOCKET] = "socket", >> + [FTYPE_TAP] = "net", >> + [FTYPE_BLK] = "blk", >> + [FTYPE_KBD] = "kbd", >> + [FTYPE_FB] = "fb", >> }; >> + >> +static char *get_type_name(unsigned int type) > > const. Okay. > >> +{ >> + if ( type < ARRAY_SIZE(file_ops) && file_ops[type] ) >> + return file_ops[type]->name; >> + >> + if ( type < ARRAY_SIZE(file_types) && file_types[type] ) >> + return file_types[type]; >> + >> + return "none"; >> +} >> #endif >> #ifdef LIBC_DEBUG >> static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *timeout) >> @@ -663,7 +759,7 @@ static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except >> if (FD_ISSET(i, set)) { \ >> if (comma) \ >> printk(", "); \ >> - printk("%d(%c)", i, file_types[files[i].type]); \ >> + printk("%d(%s)", i, get_type_name(files[i].type)); \ >> comma = 1; \ >> } \ >> } \ >> @@ -697,7 +793,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout) >> fd = pfd[i].fd; >> if (comma) >> printk(", "); >> - printk("%d(%c)/%02x", fd, file_types[files[fd].type], >> + printk("%d(%s)/%02x", fd, get_type_name(files[fd].type), >> pfd[i].events); >> comma = 1; >> } >> @@ -709,6 +805,18 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout) >> #define dump_pollfds(pfds, nfds, timeout) >> #endif >> >> +bool select_yes(int fd) >> +{ >> + return true; >> +} >> + >> +bool select_read_flag(int fd) >> +{ >> + struct file *file = get_file_from_fd(fd); >> + >> + return file->read; >> +} > > I don't see these getting used, even in a fallback case. They will be used in later patches. > >> + >> /* Just poll without blocking */ >> static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds) >> { >> @@ -762,9 +870,35 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce >> for (i = 0; i < nfds; i++) { >> switch(files[i].type) { >> default: >> + { >> + struct file_ops *ops = file_ops[files[i].type]; >> + >> + if ( ops ) >> + { >> + if ( FD_ISSET(i, readfds) ) >> + { >> + if ( ops->select_rd && ops->select_rd(i) ) >> + n++; >> + else >> + FD_CLR(i, readfds); >> + } >> + if ( FD_ISSET(i, writefds) ) >> + { >> + if ( ops->select_wr && ops->select_wr(i) ) >> + n++; >> + else >> + FD_CLR(i, writefds); >> + } >> + FD_CLR(i, exceptfds); > > Hard tab. Thanks for noting. > >> + >> + break; >> + } >> + >> if (FD_ISSET(i, readfds) || FD_ISSET(i, writefds) || FD_ISSET(i, exceptfds)) >> printk("bogus fd %d in select\n", i); >> /* Fallthrough. */ >> + } > > Is this fallthrough really appropriate, given the break in the ops case? I think this was meant for the FTYPE_SAVEFILE case. For all other file types this wouldn't make any sense. Note that in the other Mini-OS series I'm setting the savefile select callbacks appropriately. Juergen
On 11.01.22 21:05, Samuel Thibault wrote: > Juergen Gross, le mar. 11 janv. 2022 15:58:16 +0100, a ecrit: >> @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes) >> return -1; >> } >> >> +off_t lseek_default(int fd, off_t offset, int whence) >> +{ >> + switch ( whence ) >> + { > > Is there a reason for making this one a separate function, unlike others > for which you kept the code in the main function? Yes, it will be used as .lseek callback in some cases. Juergen
On 12/01/2022 07:34, Juergen Gross wrote: > On 11.01.22 19:08, Andrew Cooper wrote: >> On 11/01/2022 14:58, Juergen Gross wrote: >>> + >>> +off_t lseek_default(int fd, off_t offset, int whence); >>> +bool select_yes(int fd); >>> +bool select_read_flag(int fd); >>> + >>> LIST_HEAD(evtchn_port_list, evtchn_port_info); >>> struct evtchn_port_info { >>> @@ -183,7 +210,7 @@ struct evtchn_port_info { >>> }; >>> struct file { >>> - enum fd_type type; >>> + unsigned int type; >>> bool read; /* maybe available for read */ >>> off_t offset; >>> union { >>> @@ -199,7 +226,7 @@ struct file { >>> extern struct file files[]; >>> struct file *get_file_from_fd(int fd); >>> -int alloc_fd(enum fd_type type); >>> +int alloc_fd(unsigned int type); >>> void close_all_files(void); >>> extern struct thread *main_thread; >>> void sparse(unsigned long data, size_t size); >>> diff --git a/lib/sys.c b/lib/sys.c >>> index 323a7cd..c327247 100644 >>> --- a/lib/sys.c >>> +++ b/lib/sys.c >>> @@ -98,6 +98,39 @@ struct file files[NOFILE] = { >>> { .type = FTYPE_CONSOLE }, /* stderr */ >>> }; >>> +static struct file_ops file_ops_none = { >>> + .name = "none", >>> +}; >>> + >>> +static struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = { >>> + [FTYPE_NONE] = &file_ops_none, >>> +}; >> >> Both want to be const, because all file_ops ought to live in rodata. > > file_ops[] can't be const, it will just point to const data. Oh, of course. >>> @@ -709,6 +805,18 @@ static void dump_pollfds(struct pollfd *pfd, >>> int nfds, int timeout) >>> #define dump_pollfds(pfds, nfds, timeout) >>> #endif >>> +bool select_yes(int fd) >>> +{ >>> + return true; >>> +} >>> + >>> +bool select_read_flag(int fd) >>> +{ >>> + struct file *file = get_file_from_fd(fd); >>> + >>> + return file->read; >>> +} >> >> I don't see these getting used, even in a fallback case. > > They will be used in later patches. Yeah - I found them in later patches. It's probably worth a note in the commit message saying "provide some functions useful for file_ops in future patches", particularly as it's actually a later series which takes them (due to the ordering constraints). ~Andrew
On 12.01.22 09:19, Andrew Cooper wrote: > On 12/01/2022 07:34, Juergen Gross wrote: >> On 11.01.22 19:08, Andrew Cooper wrote: >>> On 11/01/2022 14:58, Juergen Gross wrote: >>>> + >>>> +off_t lseek_default(int fd, off_t offset, int whence); >>>> +bool select_yes(int fd); >>>> +bool select_read_flag(int fd); >>>> + >>>> LIST_HEAD(evtchn_port_list, evtchn_port_info); >>>> struct evtchn_port_info { >>>> @@ -183,7 +210,7 @@ struct evtchn_port_info { >>>> }; >>>> struct file { >>>> - enum fd_type type; >>>> + unsigned int type; >>>> bool read; /* maybe available for read */ >>>> off_t offset; >>>> union { >>>> @@ -199,7 +226,7 @@ struct file { >>>> extern struct file files[]; >>>> struct file *get_file_from_fd(int fd); >>>> -int alloc_fd(enum fd_type type); >>>> +int alloc_fd(unsigned int type); >>>> void close_all_files(void); >>>> extern struct thread *main_thread; >>>> void sparse(unsigned long data, size_t size); >>>> diff --git a/lib/sys.c b/lib/sys.c >>>> index 323a7cd..c327247 100644 >>>> --- a/lib/sys.c >>>> +++ b/lib/sys.c >>>> @@ -98,6 +98,39 @@ struct file files[NOFILE] = { >>>> { .type = FTYPE_CONSOLE }, /* stderr */ >>>> }; >>>> +static struct file_ops file_ops_none = { >>>> + .name = "none", >>>> +}; >>>> + >>>> +static struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = { >>>> + [FTYPE_NONE] = &file_ops_none, >>>> +}; >>> >>> Both want to be const, because all file_ops ought to live in rodata. >> >> file_ops[] can't be const, it will just point to const data. > > Oh, of course. > >>>> @@ -709,6 +805,18 @@ static void dump_pollfds(struct pollfd *pfd, >>>> int nfds, int timeout) >>>> #define dump_pollfds(pfds, nfds, timeout) >>>> #endif >>>> +bool select_yes(int fd) >>>> +{ >>>> + return true; >>>> +} >>>> + >>>> +bool select_read_flag(int fd) >>>> +{ >>>> + struct file *file = get_file_from_fd(fd); >>>> + >>>> + return file->read; >>>> +} >>> >>> I don't see these getting used, even in a fallback case. >> >> They will be used in later patches. > > Yeah - I found them in later patches. It's probably worth a note in the > commit message saying "provide some functions useful for file_ops in > future patches", particularly as it's actually a later series which > takes them (due to the ordering constraints). Okay, will add that note. Juergen
diff --git a/include/lib.h b/include/lib.h index 7a0546b..b40e213 100644 --- a/include/lib.h +++ b/include/lib.h @@ -66,6 +66,7 @@ #ifdef HAVE_LIBC #include <sys/queue.h> +#include <sys/stat.h> #include <stdio.h> #include <string.h> #else @@ -154,25 +155,51 @@ do { \ void sanity_check(void); #ifdef HAVE_LIBC -enum fd_type { - FTYPE_NONE = 0, - FTYPE_CONSOLE, - FTYPE_FILE, - FTYPE_XENBUS, - FTYPE_XC, - FTYPE_EVTCHN, - FTYPE_GNTMAP, - FTYPE_SOCKET, - FTYPE_TAP, - FTYPE_BLK, - FTYPE_KBD, - FTYPE_FB, - FTYPE_MEM, - FTYPE_SAVEFILE, - FTYPE_TPMFRONT, - FTYPE_TPM_TIS, +#define FTYPE_NONE 0 +#define FTYPE_CONSOLE 1 +#define FTYPE_FILE 2 +#define FTYPE_SOCKET 3 +#define FTYPE_MEM 4 +#define FTYPE_SAVEFILE 5 +#define FTYPE_FB 6 +#define FTYPE_KBD 7 +#define FTYPE_TAP 8 +#define FTYPE_BLK 9 +#define FTYPE_TPMFRONT 10 +#define FTYPE_TPM_TIS 11 +#define FTYPE_XENBUS 12 +#define FTYPE_GNTMAP 13 +#define FTYPE_EVTCHN 14 +#define FTYPE_XC 15 +#define FTYPE_N 16 +#define FTYPE_SPARE 16 + +typedef int file_read_func(int fd, void *buf, size_t nbytes); +typedef int file_write_func(int fd, const void *buf, size_t nbytes); +typedef off_t file_lseek_func(int fd, off_t offset, int whence); +typedef int file_close_func(int fd); +typedef int file_fstat_func(int fd, struct stat *buf); +typedef int file_fcntl_func(int fd, int cmd, va_list args); +typedef bool file_select_func(int fd); + +struct file_ops { + const char *name; + file_read_func *read; + file_write_func *write; + file_lseek_func *lseek; + file_close_func *close; + file_fstat_func *fstat; + file_fcntl_func *fcntl; + file_select_func *select_rd; + file_select_func *select_wr; }; +unsigned int alloc_file_type(struct file_ops *ops); + +off_t lseek_default(int fd, off_t offset, int whence); +bool select_yes(int fd); +bool select_read_flag(int fd); + LIST_HEAD(evtchn_port_list, evtchn_port_info); struct evtchn_port_info { @@ -183,7 +210,7 @@ struct evtchn_port_info { }; struct file { - enum fd_type type; + unsigned int type; bool read; /* maybe available for read */ off_t offset; union { @@ -199,7 +226,7 @@ struct file { extern struct file files[]; struct file *get_file_from_fd(int fd); -int alloc_fd(enum fd_type type); +int alloc_fd(unsigned int type); void close_all_files(void); extern struct thread *main_thread; void sparse(unsigned long data, size_t size); diff --git a/lib/sys.c b/lib/sys.c index 323a7cd..c327247 100644 --- a/lib/sys.c +++ b/lib/sys.c @@ -98,6 +98,39 @@ struct file files[NOFILE] = { { .type = FTYPE_CONSOLE }, /* stderr */ }; +static struct file_ops file_ops_none = { + .name = "none", +}; + +static struct file_ops *file_ops[FTYPE_N + FTYPE_SPARE] = { + [FTYPE_NONE] = &file_ops_none, +}; + +unsigned int alloc_file_type(struct file_ops *ops) +{ + unsigned int i; + + pthread_mutex_lock(&fd_lock); + + for ( i = FTYPE_N; i < ARRAY_SIZE(file_ops) && file_ops[i]; i++ ); + BUG_ON(i == ARRAY_SIZE(file_ops)); + file_ops[i] = ops; + + pthread_mutex_unlock(&fd_lock); + + printk("New file type \"%s\"(%u) allocated\n", ops->name, i); + + return i; +} + +static struct file_ops *get_file_ops(unsigned int type) +{ + if ( type >= ARRAY_SIZE(file_ops) || !file_ops[type] ) + return &file_ops_none; + + return file_ops[type]; +} + struct file *get_file_from_fd(int fd) { if ( fd < 0 || fd >= NOFILE ) @@ -108,7 +141,7 @@ struct file *get_file_from_fd(int fd) DECLARE_WAIT_QUEUE_HEAD(event_queue); -int alloc_fd(enum fd_type type) +int alloc_fd(unsigned int type) { int i; pthread_mutex_lock(&fd_lock); @@ -249,6 +282,11 @@ int isatty(int fd) int read(int fd, void *buf, size_t nbytes) { + struct file_ops *ops = get_file_ops(files[fd].type); + + if ( ops->read ) + return ops->read(fd, buf, nbytes); + switch (files[fd].type) { case FTYPE_SAVEFILE: case FTYPE_CONSOLE: { @@ -328,6 +366,11 @@ int read(int fd, void *buf, size_t nbytes) int write(int fd, const void *buf, size_t nbytes) { + struct file_ops *ops = get_file_ops(files[fd].type); + + if ( ops->write ) + return ops->write(fd, buf, nbytes); + switch (files[fd].type) { case FTYPE_SAVEFILE: { int ret = 0, tot = nbytes; @@ -370,8 +413,45 @@ int write(int fd, const void *buf, size_t nbytes) return -1; } +off_t lseek_default(int fd, off_t offset, int whence) +{ + switch ( whence ) + { + case SEEK_SET: + files[fd].offset = offset; + break; + + case SEEK_CUR: + files[fd].offset += offset; + break; + + case SEEK_END: + { + struct stat st; + int ret; + + ret = fstat(fd, &st); + if ( ret ) + return -1; + files[fd].offset = st.st_size + offset; + break; + } + + default: + errno = EINVAL; + return -1; + } + + return files[fd].offset; +} + off_t lseek(int fd, off_t offset, int whence) { + struct file_ops *ops = get_file_ops(files[fd].type); + + if ( ops->lseek ) + return ops->lseek(fd, offset, whence); + switch(files[fd].type) { #ifdef CONFIG_BLKFRONT case FTYPE_BLK: @@ -393,28 +473,7 @@ off_t lseek(int fd, off_t offset, int whence) return (off_t) -1; } - switch (whence) { - case SEEK_SET: - files[fd].offset = offset; - break; - case SEEK_CUR: - files[fd].offset += offset; - break; - case SEEK_END: - { - struct stat st; - int ret; - ret = fstat(fd, &st); - if (ret) - return -1; - files[fd].offset = st.st_size + offset; - break; - } - default: - errno = EINVAL; - return -1; - } - return files[fd].offset; + return lseek_default(fd, offset, whence); } int fsync(int fd) { @@ -425,8 +484,15 @@ int fsync(int fd) { int close(int fd) { int res = 0; + struct file_ops *ops = get_file_ops(files[fd].type); printk("close(%d)\n", fd); + if ( ops->close ) + { + res = ops->close(fd); + goto out; + } + switch (files[fd].type) { default: break; @@ -497,6 +563,7 @@ int close(int fd) return -1; } + out: memset(files + fd, 0, sizeof(struct file)); files[fd].type = FTYPE_NONE; return res; @@ -521,7 +588,13 @@ int stat(const char *path, struct stat *buf) int fstat(int fd, struct stat *buf) { + struct file_ops *ops = get_file_ops(files[fd].type); + init_stat(buf); + + if ( ops->fstat ) + return ops->fstat(fd, buf); + switch (files[fd].type) { case FTYPE_SAVEFILE: case FTYPE_CONSOLE: @@ -587,6 +660,18 @@ int fcntl(int fd, int cmd, ...) { long arg; va_list ap; + int res; + struct file_ops *ops = get_file_ops(files[fd].type); + + if ( ops->fcntl ) + { + va_start(ap, cmd); + res = ops->fcntl(fd, cmd, ap); + va_end(ap); + + return res; + } + va_start(ap, cmd); arg = va_arg(ap, long); va_end(ap); @@ -640,18 +725,29 @@ int closedir(DIR *dir) /* We assume that only the main thread calls select(). */ #if defined(LIBC_DEBUG) || defined(LIBC_VERBOSE) -static const char file_types[] = { - [FTYPE_NONE] = 'N', - [FTYPE_CONSOLE] = 'C', - [FTYPE_XENBUS] = 'S', - [FTYPE_XC] = 'X', - [FTYPE_EVTCHN] = 'E', - [FTYPE_SOCKET] = 's', - [FTYPE_TAP] = 'T', - [FTYPE_BLK] = 'B', - [FTYPE_KBD] = 'K', - [FTYPE_FB] = 'G', +static const char *file_types[] = { + [FTYPE_NONE] = "none", + [FTYPE_CONSOLE] = "console", + [FTYPE_XENBUS] = "xenbus", + [FTYPE_XC] = "ctrl", + [FTYPE_EVTCHN] = "evtchn", + [FTYPE_SOCKET] = "socket", + [FTYPE_TAP] = "net", + [FTYPE_BLK] = "blk", + [FTYPE_KBD] = "kbd", + [FTYPE_FB] = "fb", }; + +static char *get_type_name(unsigned int type) +{ + if ( type < ARRAY_SIZE(file_ops) && file_ops[type] ) + return file_ops[type]->name; + + if ( type < ARRAY_SIZE(file_types) && file_types[type] ) + return file_types[type]; + + return "none"; +} #endif #ifdef LIBC_DEBUG static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, struct timeval *timeout) @@ -663,7 +759,7 @@ static void dump_set(int nfds, fd_set *readfds, fd_set *writefds, fd_set *except if (FD_ISSET(i, set)) { \ if (comma) \ printk(", "); \ - printk("%d(%c)", i, file_types[files[i].type]); \ + printk("%d(%s)", i, get_type_name(files[i].type)); \ comma = 1; \ } \ } \ @@ -697,7 +793,7 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout) fd = pfd[i].fd; if (comma) printk(", "); - printk("%d(%c)/%02x", fd, file_types[files[fd].type], + printk("%d(%s)/%02x", fd, get_type_name(files[fd].type), pfd[i].events); comma = 1; } @@ -709,6 +805,18 @@ static void dump_pollfds(struct pollfd *pfd, int nfds, int timeout) #define dump_pollfds(pfds, nfds, timeout) #endif +bool select_yes(int fd) +{ + return true; +} + +bool select_read_flag(int fd) +{ + struct file *file = get_file_from_fd(fd); + + return file->read; +} + /* Just poll without blocking */ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds) { @@ -762,9 +870,35 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce for (i = 0; i < nfds; i++) { switch(files[i].type) { default: + { + struct file_ops *ops = file_ops[files[i].type]; + + if ( ops ) + { + if ( FD_ISSET(i, readfds) ) + { + if ( ops->select_rd && ops->select_rd(i) ) + n++; + else + FD_CLR(i, readfds); + } + if ( FD_ISSET(i, writefds) ) + { + if ( ops->select_wr && ops->select_wr(i) ) + n++; + else + FD_CLR(i, writefds); + } + FD_CLR(i, exceptfds); + + break; + } + if (FD_ISSET(i, readfds) || FD_ISSET(i, writefds) || FD_ISSET(i, exceptfds)) printk("bogus fd %d in select\n", i); /* Fallthrough. */ + } + case FTYPE_CONSOLE: if (FD_ISSET(i, readfds)) { if (xencons_ring_avail(files[i].dev)) @@ -842,7 +976,7 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce printk("%d(%d): ", nb, sock_n); for (i = 0; i < nfds; i++) { if (nbread[i] || nbwrite[i] || nbexcept[i]) - printk(" %d(%c):", i, file_types[files[i].type]); + printk(" %d(%c):", i, get_type_name(files[i].type)); if (nbread[i]) printk(" %dR", nbread[i]); if (nbwrite[i])
Introduce file type specific function vectors for all the needed file operations which are file type specific in order to prepare replacing the large switch statements in each generic file function. Add a function to allocate a new file type dynamically in order to prepare removing direct dependencies to external components in the future. For this reason switch the file type from an enum to an unsigned int. Prepare removal of some statically defines file types by putting them at the end of the defined list. Change the debug output for the file type from a single character to a string in order to support a future removal of the file_types[] array. Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - new patch --- include/lib.h | 65 +++++++++++----- lib/sys.c | 208 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 217 insertions(+), 56 deletions(-)