fcntl() and required handlers Re: ss-20000929

[Chris Johns]

Joel Sherrill wrote:
> 
> I have comments interspersed but this change is from
> Chris Johns and I would like him to comment as well.  Here
> is the ChangeLog entry so it applies to other entries
> as well:
> 
> revision 1.14
> date: 2000/08/11 20:04:27;  author: joel;  state: Exp;  lines: +11 -5
> 2000-08-11  Chris Johns <ccj at acm.org>
> 
>         * libc/chmod.c: Return ENOTSUP if filesystem does not have
> handler.
>         * libc/eval.c: Ditto.
>         * libc/fcntl.c: Ditto.
>         * libc/fsync.c: Ditto.
>         * libc/ioctl.c: Ditto.
>         * libc/ioman.c: Ditto.
>         * libc/link.c: Ditto.
>         * libc/memfile.c: Ditto.
>         * libc/mknod.c: Ditto.
>         * libc/symlink.c: Ditto.
>         * libc/libio.h(rtems_filesystem_dev_major_t): New macro.
>         * libc/libio.h(rtems_filesystem_dev_minor_t): New macro.
> 
> Antti P Miettinen wrote:
> >
> > I now know why fdopen fails but I'd like some guidance how to fix it.
> > Newlib fdopen does a fcntl with F_GETFL. RTEMS fcntl reacts with:
> >
> >     case F_GETFL:        /* more flags (cloexec) */
> >       ret = rtems_libio_to_fcntl_flags( iop->flags );
> >       break;
> >
> > and in 4.5.0 beta 3 a continues with
> >
> >   if ((ret >= 0) && iop->handlers->fcntl) {
> >     int err = (*iop->handlers->fcntl)( cmd, iop );
> >     if (err) {
> >       errno = err;
> >       ret = -1;
> >     }
> >   }
> >
> > but in ss-20000929 with
> >
> >   if (ret >= 0) {
> >     if (iop->handlers->fcntl_h) {
> >       int err = (*iop->handlers->fcntl_h)( cmd, iop );
> >       if (err) {
> >         errno = err;
> >         ret = -1;
> >       }
> >     }
> >     else {
> >       errno = ENOTSUP;
> >       ret = -1;
> >     }
> >   }
> >
> > resulting in failure.
> 
> Which filesystem is this with?
> 
> My first thought is that you are correct.  This code was not
> 100% correct before either.  I think this also applies to
> F_GETFD based on my reading of that man page.  There is
> no reason to go to the filesystem/file specific handler
> for a get flags.
> 
> The intention of this code is to let the handler get a shot
> at doing something when the generic code path worked.  So
> for example, if you are setting flags, the filesystem handler
> gets a chance to react.
> 
> > Couldn't fcntl just return at the switch/case F_GETFL? I suppose
> > devices might want to have their own fcntl_h to react to some things
> > in a special way but isn't for example F_GETFL always handled directly
> > and completely in fcntl()? Or should there always be at least a dummy
> > fcntl_h for fcntl() to be allowed to return ok?
> 
> I think this argument is true for both F_GETFD and F_GETFL.
> 
> The way the code is currently written, every filesystem has to provide
> at least a dummy fcntl_h handler.  If it does not require one, then
> why should it be forced to return ENOTSUP.
> 

Because I did not know better :-)

I made these changes as the miniIMFS broke as it referenced a null
pointer.  Check `miniimfs_init.c' to see the table entries which are 0.
I plugged the hole but it looks like the code needs more work to be bad
correct.

-- 
 Chris Johns, mailto:cjohns at cybertec.com.au mailto:ccj at acm.org