posixmqcontrol: manage POSIX message queues

[unitrunker_gmail.com]

Summary

This review introduces a new utility - in the spirit of posixshmcontrol - for managing POSIX message queues.

Summary of features:

1 create one or more new queues with the following in common:

• maximum message size.
• maximum queue depth.
• mode bits (rwx).
• group and user ownership (by ID or name).
• blocking or non-blocking I/O.

2. update all of the above attributes to existing queues.
3. inspect the above attributes on one or more existing queues.
4. send one or more messages to one or more named queues with optional priority.
5. receive a message from a named queue.
6. remove one or more named queues.

This utility requires the mqueuefs kernel module to be loaded. It does NOT require or make use of a mounted mqueuefs.

Justification

• Create all required queues following boot.
• De-couple queue administration from application code.
• Queue depthing monitoring.
• Access control management with or without a mounted mqueuefs file system.

More details can be found in the manual page.

Test Plan

A. sanity checks on queue names - script posixmqcontroltestsane.sh

1. must start with leading slash '/'
2. must not contain more than one slash '/'
3. must not be longer than NAME_MAX characters.

B. create, info, recv, send, and rm subcommands are tested for single queue operation - script posixmqcontroltest8x64.sh

1. verify info on non-existing queue returns an error.
2. create one queue.
3. verify queue exists.
4. send eight distinct messages - with increasing priority.
5. verify queue has eight messages waiting.
6. drain each message from queues - verifying expected order based on priority.
7. verify queue is empty.
8. remove queue.

C. create, info, send, and rm are tested for mutiple queue operations. send is tested for multiple messages - script posixmqcontroltest8qs.sh

1. verify 'info' on non-existing queue returns an error.
2. create eight queues in one 'create' operation.
3. verify eight queues exist in one 'info' operation.
4. send one message to all eight queues in one 'send' operation.
5. verify each queue has one message waiting as one 'info' operation.
6. drain one message from each of eight queues (one at a time).
7. verify each queue is empty as one 'info' operation.
8. remove all queues as one 'rm' operation.

[kib]

Please start by reading style(9) and adjusting the code accordingly.

I did not read the algorithms.

usr.bin/posixmqcontrol/posixmqcontrol.c

3

Is it?

40

Includes should be ordered alphabetically, sys/ first, userspace second.

43

All of that is supplied by proper C headers like stdbool.h. Then, why do you need nullptr instead of NULL for C?

46

The comments should be C89. That said, I do not see much use in comment repeating a variable name in the sentence.

55

should it be long or size_t?

61

gid_t

65

uid_t

68

Another useless comment.

73

and so on

269–270

This is perhaps just !STAILQ_EMPTY().

462

It is weird mix of perror message going to stderr and then mostly useless error numeric repeated to stdout.

Read about err(3).

556

What is the purpose of __BSD_VISIBLE checks? First, this symbol is private for system headers. Second, the utility comes to the FreeBSD base, so it is definitely compiled in default compilation environment, or at least you control it with your own Makefile.

596

This is very strange, you are returning errno namespace numbers as utility exit codes.

[imp]

tools/build/checkstyle9.pl
Or clang-format from an llvm port

[unitrunker_gmail.com]

Thank you - @kib and @imp - for the feedback and comments. This utility started out as a C++ program before being converted to C. Will work through the comments one at a time.

[unitrunker_gmail.com]

usr.bin/posixmqcontrol/posixmqcontrol.c

556

What is the purpose of __BSD_VISIBLE checks? First, this symbol is private for system headers. Second, the utility comes to the FreeBSD base, so it is definitely compiled in default compilation environment, or at least you control it with your own Makefile.

This symbol brackets mq_getfd_np in <mqueue.h>. It has been available since 11.x-RELEASE and is currently used by rust. There is an open bug requesting it be documented.

See https://bugs.freebsd.org/bugzilla//show_bug.cgi?id=273230

https://github.com/rust-lang/libc/pull/1308

It is important to point out that half the functionality of this utility relies on being able to get the file descriptor from the mqd_t handle. fstat and fchown won't work without it.

[unitrunker_gmail.com]

usr.bin/posixmqcontrol/posixmqcontrol.c

55

This matches the corresponding struct in <mqueue.h> for mq_maxmsg and mq_msgsize.

struct mq_attr {
long mq_flags; /* Message queue flags. */
long mq_maxmsg; /* Maximum number of messages. */
long mq_msgsize; /* Maximum message size. */
long mq_curmsgs; /* Number of messages currently queued. */
long __reserved[4]; /* Ignored for input, zeroed for output */
};

[kib]

usr.bin/posixmqcontrol/posixmqcontrol.c

556

What you wrote does not answer my question. Unless you enable restricted compilation environment, all BSD/FreeBSD specific (non-standard) symbols are available.

[unitrunker_gmail.com]

usr.bin/posixmqcontrol/posixmqcontrol.c

556

Short answer: portability.

@kib - the #ifdef is there in case the symbol is not defined. I could not tell from <mqueue.h> when the symbol is defined and under what circumstances it is guaranteed to be present.

If you can, please provide a cite for "restricted compilation environment". I searched the wiki and handbook but did not find a match.

If the symbol is always defined, then no harm. This also allows the code to be usable in other POSIX friendly environments outside of FreeBSD.

A small backstory - someone recommended 'posixshmcontrol' on stack overflow but they were rebuked because posixshmcontrol was considered a FreeBSD only tool. With very little effort that tool could be made portable. I've made some effort here to give this tool some degree of portability.

[unitrunker_gmail.com]

warnx/warnc logging.
convert errnos to exit codes from <sysexits.h>
replace BSD_VISIBLE with FreeBSD__ (for portability).
make use of <stdbool.h>
drop use of nullptr.
remove superfluous comments.
style-9 compliant comments.

[unitrunker_gmail.com]

In D43813#999429, @imp wrote:

tools/build/checkstyle9.pl
Or clang-format from an llvm port

From checkstyle9.pl: total: 141 errors, 48 warnings, 689 lines checked

Working through those now.

[unitrunker_gmail.com]

posixmqcontrol.c has no obvious style problems and is ready for submission.

[unitrunker_gmail.com]

applied checkstyle9.pl feedback.

[kib]

usr.bin/posixmqcontrol/posixmqcontrol.c

46

Indentation should be done with tab (displayed as 8 spaces). Continuation lines should have +4 spaces.

73

76

The element_size var is useless. The cast of malloc return value is not needed.

105

There should be a blank line between local vars declaration and statements parts.

[kib]

usr.bin/posixmqcontrol/posixmqcontrol.c

694

This comment is not useful, it repeats a common knowledge. That said, you print out modes in octal instead of ls long format. Might be, it should be changed to that.

[unitrunker_gmail.com]

• unit test updated for new exit codes.
• indentation for continuation lines
• use tabs for indentation

[unitrunker_gmail.com]

more style(9) conformance

• don't treat character pointer deref as boolean
• return types on separate lines.

[unitrunker_gmail.com]

Following @kib's suggestion, utility was updated to format the mode bits ... and I just spotted a mistake. Stand by ...

[unitrunker_gmail.com]

copy/paste typo caused mode other 'x' bit to appear as 'r'

[unitrunker_gmail.com]

Updated the mode bits formatting. Example:

$ posixmqcontrol info -q /A -q /B -q /C -q /D -q /E | grep 'MODE:'
MODE: -rwxr-sr-x
MODE: -rwsr-xr-x
MODE: -rw-r-Sr-x
MODE: -rwSr--r-x
MODE: -rwxr-xr-x

Compare to ...

$ ls -l /mqfs
total 0
-rwxr-sr-x  1 unitrunker wheel 0 Feb 11 12:55 A
-rwsr-xr-x  1 unitrunker wheel 0 Feb 11 12:55 B
-rw-r-Sr-x  1 unitrunker wheel 0 Feb 11 12:55 C
-rwSr--r-x  1 unitrunker wheel 0 Feb 11 12:55 D
-rwxr-xr-x  1 unitrunker wheel 0 Feb 11 14:22 E

[kib]

usr.bin/posixmqcontrol/posixmqcontrol.1

60

New sentence new line.

usr.bin/posixmqcontrol/posixmqcontrol.c

76

102

This is a continuation line, it should get +4 spaces indent instead of tab

145

+4 spaces not tab

152

154

same

183

I suggest to add a check for malloc() result in malloc_element() and err(1, "malloc") on allocation failure.

202

Fix indent pls

274

Blank line before

276

Indent

286

309

364

indent

366

368

blank line after

426

I suggest to use designated initializers

463

blank line after

540

555

Again use designated initializers

565

indent

572

No need to initialize status

581

indent. Consider using constants from sys/stat.h

[unitrunker_gmail.com]

• permission bits.
• continuation uses four spaces (not tab).
• exit code for malloc failure.

[unitrunker_gmail.com]

I'm having trouble aligning kib's comments with the patch files. I'll submit a new revision.

[unitrunker_gmail.com]

no single line functions

[unitrunker_gmail.com]

New review here:

https://reviews.freebsd.org/D43845

[unitrunker_gmail.com]

Abandoned.