iovec: macros to manipulate len and base together

[brooks]

Summary

A set of convenience macros to initialize struct iovec's and increment
the base and length together.

IOVEC_INIT - sets iov_base and iov_len
IOVEC_INIT_STR - takes a string and sets iov_len to strlen + 1
IOVEC_INIT_OBJ - takes an object and sets iov_len to sizeof obj
IOVEC_ADVANCE - increments iov_base and decrements iov_len

On CheriBSD these present the opportunity to insert more-precise bounds
on objects and hide differences in casts in hybrid kernels (where
some, but no all pointers are capabilities). Here in FreeBSD the
resulting code is tidier, particularly in the IOVEC_ADVANCE case where
the need to cast iov_base to (char *) is avoided.

[des]

sys/sys/_iovec.h

53

It isn't obvious to me that we always, or even mostly, want to include the null terminator when we pass a string through an iovec. Therefore, I suggest renaming the macro to make it clear that it does account for the terminator.

[kib]

This file is included into sys/socket.h and sys/uio.h, we do not want to pollute the namespace.

Also I think it is useful to utilize expr-statement extension ({ ... }) to prevent multi-evaluation of the macro args.

[brooks]

In D45422#1036183, @kib wrote:

This file is included into sys/socket.h and sys/uio.h, we do not want to pollute the namespace.

We can limit the pollution, but there's no way (and no value) in eliminating it. We could choose to make them all _IOVEC and probably should put it under __BSD_VISIBLE. I'll do the latter now.

Also I think it is useful to utilize expr-statement extension ({ ... }) to prevent multi-evaluation of the macro args.

I'll do that.

sys/sys/_iovec.h

53

Empirically there is one case that doesn't want the terminator and 15 that do (5 of which are in the same file as the single case without the terminator). Perhaps IOVEC_INIT_STRNUL() would be a better name?

[brooks]

• Guard macros with __BSD_VISIBLE (cdefs.h included via _types.h)
• Switch to extension macros
• Document duplicate expansion in the _OBJ case. It isn't obviously possible to avoid.

[jrtc27]

sys/sys/_iovec.h

63

Eh, it's only evaluated if obj is a VLA; sizeof doesn't otherwise evaluate the expression

[jrtc27]

I don't think any of these need to be __extension__({ ... }), you're not using them as an expression, you're using them as a statement, so do { ... } while (0) is perfectly fine.

sys/sys/_iovec.h

48–52

Could use a compound literal here

[brooks]

In D45422#1036229, @jrtc27 wrote:

I don't think any of these need to be __extension__({ ... }), you're not using them as an expression, you're using them as a statement, so do { ... } while (0) is perfectly fine.

I've been slightly tempted to make the API return the iovecp so you could turn:

	auio.uio_iov = &aiov;
	auio.uio_iovcnt = 1;
	aiov.iov_base = (void *)&snaplistsize;
	aiov.iov_len = sizeof(snaplistsize);

into

auio.uio_iov = IOVEC_INIT(&aiov, &snaplistsize, sizeof(snaplistsize));
auio.uio_iovcnt = 1;

which would benefit from __extension__, but I've not convinced myself it's an improvement (it's certainly not general given that uio_iov is an array in general.

[brooks]

Implement IOVEC_INIT with compound literal

[kib]

sys/sys/_iovec.h

70

Use of KASSERT means that the macros are kernel-only. Then why expose them to userspace at all?

[brooks]

Move macros under _KERNEL. I've verfied upstream that nothing in
CheriBSD needs this in userspace so confine it to the kernel now.

[kib]

Do you plan to introduce use of the macros into FreeBSD?

sys/sys/_iovec.h

61

[brooks]

In D45422#1036267, @kib wrote:

Do you plan to introduce use of the macros into FreeBSD?

Yes. See D45423 as an example of converting UFS. I plan to convert all the non-contrib code and plan to merge by subsystem to allow backouts should I make mistakes (it's all been in CheriBSD for years, but I've got to merge by hand.)

[jrtc27]

sys/sys/_iovec.h

63

It would be good to get the comment to be more correct

63

This doesn't need __extension__({ ... }), it's a single statement

73

All the INIT macros end up evaluating to the iovec *, but this one evaluates to its new iov_base. Is that intended? I'd rather not expose an accidental API.

[des]

sys/sys/_iovec.h

53

STRNUL or CSTR or STRZ, cf. Karlton's Law

[brooks]

IOVEC_INIT_STR -> IOVEC_INIT_CSTR

Don't wrap single statements

Use do {} while(0) rather than extension for simplicity (I'd hoped
extension might supress -Wshadow since it's a pseudo function, but
it does not and we don't need the return affordance so skip it.)

Put potentially shadowing variables in the implementation namespace
(__).

[jrtc27]

sys/sys/_iovec.h

61

This comment is still incorrect for the vast majority of uses.

[brooks]

Drop comment about double eval