Re: [PATCH v2] man/man2: introduce man page for file_getattr/file_setattr syscalls

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2912 bytes --]

[CC += PERFORMANCE EVENTS SUBSYSTEM maintainers]

Hi Andrey,

> Date: 2026-09-11 13:17:12+0200
> From: Andrey Albershteyn <aalbersh@kernel.org>
>
> Hi Alejandro,
> 
> Thanks for the review, I will apply your suggestions, responses to
> some of the questions below:
> 
[...]
> > > +The
> > > +.I flags
> > > +argument is a bit mask, available flags are:
> > 
> > The usual language we use for this is:
> > 
> > 	The
> > 	.I flags
> > 	argument contains
> > 	a bitwise OR of zero or more of the following constants:
> > 
> > See for example readv(2).
> 
> I will use "constants" then (the readv uses "following flags")

Ok.

> > There are some minor variations of this in other pages, and I should
> > make them more uniform.
> > 
[...]
> > > +.TP
> > > +.B EBADF
> > > +.I pathname
> > > +is relative but
> > > +.I dirfd
> > > +is neither
> > > +.B AT_FDCWD
> > > +nor a valid file descriptor.
> > > +.TP
> > > +.B EBADF
> > > +.I pathname
> > > +is an empty string,
> > > +.B AT_EMPTY_PATH
> > > +was specified,
> > > +but
> > > +.I dirfd
> > > +is an invalid file descriptor.
> > 
> > I was wondering: is it valid to specify AT_EMPTY_PATH, use an empty
> > string, and use AT_FDCWD as the dirfd?  That should act on the current
> > working directory itself, right?  Or is that not supported?
> 
> Yes, this is valid combination. I can describe this case.

Thanks!  I think using wording similar to the one above would be enough:

	but
	.I dirfd
	is neither
	.B AT_FDCWD
	nor a valid file descriptor.

[...]
> > > +.TP
> > > +.B EINVAL
> > > +.I size
> > > +is smaller than
> > > +.BR FILE_ATTR_SIZE_VER0 .
> > 
> > perf_event_open(2) reports E2BIG for a size smaller than
> > PERF_ATTR_SIZE_VER0.  This seems unnecessarily inconsistent.  I'm not
> > sure which I'd say is more appropriate, but I'd expect them to be
> > consistent.  I mentioned perf_event_open(2) because that's the only page
> > that has a *_VER0 constant and documents an error if a size is smaller
> > than it.  There's also mount_setattr(2) which documents
> > MOUNT_ATTR_SIZE_VER0, but it's not documented in ERRORS.
> > 
> > I think kernel maintainers should have a look at the different APIs that
> > have such a value, and discuss whether the error codes should be made
> > uniform retroactively, or whether we should accept the existing
> > divergence but decide on an error code for new APIs.
> > 
> > I've CCed linux-api@.
> 
> The MOUNT_ATTR_SIZE_VER0 also returns an EINVAL

Thanks!  I'll document that.

> I'm in favor of EINVAL as E2BIG is probably a more confusing naming
> for the too small argument.

I think I agree.

I've CCd the maintainers of perf_event_open(2).  Can perf_event_open(2)
be changed to report EINVAL?  Or is that mistake set in stone?


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]