Re: CVS commit: src/sys/kern

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Thu, 29 Sep 2022 16:47:06 -0000 (UTC)
    From:        christos%astron.com@localhost (Christos Zoulas)
    Message-ID:  <th4i6a$aq$1%ciao.gmane.io@localhost>

  | I think that the way to go is to:
  |
  | 1. Do the fd_set_exclose() in fd_affix(). That will remove most of the calls
  |    to fd_set_exclose() *and* the open-coded versions of it.
  | 2. Move the open_setfp locking initialization code to fd_affix() and do it
  |    if fp->f_type == DTYPE_VNODE. This should enable locking in all the
  |    appropriate cloners.

I initially intended to reply and say that decisions where to put stuff
like that were for someone else (you, dholland, ...) rather than me, as
I haven't played around much at this level since before vnodes existed.

But I have been thinking about it, and I disagree with that approach.

fp_affix() has a job to do, and should be left to do it, without being
burdened by applying weird side effects, sometimes.   The "do one thing
and do it well" philosophy applies to more than the commands.

eg: currently fd_affix() is a void func, but to handle the lock flags
it would need to be able to fail, and return an error code.  It would
also need to be able to sleep.   That's just wrong.

O_CLOEXEC and O_??LOCK are high level open() flags, and deserve to be
handled somewhere near the upper levels of the open syscall handling,
not buried in some utility function.

kre