On 2025-09-04 22:14, Konstantin Belousov wrote:
On Thu, Sep 04, 2025 at 09:43:13PM -0700, James Gritton wrote:
On 2025-09-04 16:43, Konstantin Belousov wrote:
> jd = malloc(sizeof(*jd), M_JAILDESC, M_WAITOK | M_ZERO);
> error = falloc_caps(td, &fp, fdp, 0, NULL);
> finit(fp, priv_check_cred(fp->f_cred, PRIV_JAIL_SET) == 0
> ? FREAD | FWRITE : FREAD, DTYPE_JAILDESC, jd, &jaildesc_ops);
> ^^^^^^^^^^^ '?' should be placed on the previous line
I wasn't aware of this requirement; style(9) is silent on it.
In fact style(9) contains the explicit requirement:
If you have to wrap a long statement, put the operator at the end
of the
line.
There are a lot more of this pattern repeated in the commit.
Ah, yes it says that. I was scanning examples looking for the "?",
and missed the general statement. I don't think it's the best move
for the ?: operator in particular, but I appreciate consistency in
style. I can change that in kern_jaildesc.c, but it's a little
trickier in kern_jail.c since it's already replete with me having done
it wrong over the years.
> Generated files should have been committed as a follow-up, not in the
> same commit as written code.
The FreeBSD Wiki explicitly allows it in the same commit.
I always objected against this practice. For instance, the commit
message
for this commit is even less useful because most of the limit was
filled
with the auto-generated stuff, instead of the code. Same for reading
the
commits with log.
Could you please point me to the wiki page?
https://wiki.freebsd.org/AddingSyscalls#Committing
> jaildesc_find() returns EBADF when passed file type is not DTYPE_JAIL.
> Normally EBADF means that the object underlying the file is invalidated,
> like vnode is reclaimed, tty is revoked, etc. For the wrong type, EINVAL
> should be returned.
That's part of the code that I lifted from process descriptors, nearly
identical to procdesc_find. A check of other c_type checks shows
EBADF isn't uncommon.
So procdesc is wrong as well, I think.
The existing code base is quite inconsistent. I some EINVAL, some
EBADF (procdesc, kqueue, fcntl), also EPERM, ENODEV, ENOTSUPP, EPIPE,
and ENOTSOCK. EINVAL is the most common, but there are enough EBADF
that I feel I can keep it.
- Jamie
