Re: [PATCH v2 17/18] mini-os: use function vectors instead of switch for file operations

[Juergen Gross <jgross@xxxxxxxx>]

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

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature