Doxygen param "in/out" attributes not matching function behavior

[Jens Schweikhardt]

It would seem that a number of doxygen comments in the source code do not match the function's behavior.

Example: /cpukit/include/rtems/rtems/message.h

171	/**
172	 * @brief RTEMS Message Queue Receive
173	 *
174	 * This routine implements the rtems_message_queue_receive directive.
175	 * This directive is invoked when the calling task wishes to receive
176	 * a message from the message queue indicated by ID. The received
177	 * message is to be placed in buffer. If no messages are outstanding
178	 * and the option_set indicates that the task is willing to block,
179	 * then the task will be blocked until a message arrives or until,
180	 * optionally, timeout clock ticks have passed.
181	 *
182	 * @param[in] id is the queue id
183	 * @param[in] buffer is the pointer to message buffer
184	 * @param[in] size is the size of message receive
185	 * @param[in] option_set is the options on receive
186	 * @param[in] timeout is the number of ticks to wait
187	 *
188	 * @retval This method returns RTEMS_SUCCESSFUL if there was not an
189	 *         error. Otherwise, a status code is returned indicating the
190	 *         source of the error.
191	 */
192	rtems_status_code rtems_message_queue_receive(
193	  rtems_id        id,
194	  void           *buffer,
195	  size_t         *size,
196	  rtems_option    option_set,
197	  rtems_interval  timeout
198	);

Since the parameters "buffer" and "size" are output parameters, the comment should say

183	 * @param[out] buffer is the pointer to message buffer
184	 * @param[out] size is the size of message receive

There are other functions where this is inconsistent, so a look at all functions taking pointer parameters is likely a good idea.

[Joel Sherrill]

This method was not the best example to pick to back up your case. The size parameter is used as *size for the size of the caller's buffer and filled in on output with the size of the message actually received.

​https://git.rtems.org/rtems/tree/cpukit/score/src/coremsgseize.c#n46

This particular case is indeed in/out.

Others may indicate the thinking that the pointer was an input and *pointer was the output.

[Jens Schweikhardt]

OK, from the description it was not clear that size is actually [inout]. Maybe there's room for improvement as well.

As for "Others may indicate" I would argue that in that case "in" is redundant, or even pointless, since in C parameters are always passed by value, so can only be "in" by language definition.

Nit-pick: the coremsgseize.c you reference says "@brief Size" where it meant "Seize". :-)

[Joel Sherrill]

Replying to Jens Schweikhardt:

OK, from the description it was not clear that size is actually [inout]. Maybe there's room for improvement as well.

I think this may be the only case in the Classic API where the value of a *argument is expected to have a value set by the caller.. So I was picking a bit at the unlucky choice for an example. :)

The fact you landed on this one as an example without realizing that may indicate the documentation could be improved. :(

This still doesn't answer the question of how an output parameter via pointer should be annotated. The pointer has a value on input (e.g. in) but the *pointer is where the output goes.

And the Doxygen for this method doesn't indicate that the buffer and *size will be set on success. :(

As for "Others may indicate" I would argue that in that case "in" is redundant, or even pointless, since in C parameters are always passed by value, so can only be "in" by language definition.

Yes. This is true. But the personality of the people who did this are anal about consistency. If you say out or inout for some, what is the output for those where nothing is said in the input? Does it look like a mistake not to include in annotation? It is redundant but makes things consistent.

Nit-pick: the coremsgseize.c you reference says "@brief Size" where it meant "Seize". :-)

Got it. I fixed that locally and will push a commit when I catch my next round of patches.

Interestingly, Seize breaks the rule of "i before e except after c"

[Sebastian Huber]

The use of these [in], [out] and [in,out] specifiers is inconsistent and no clear guideline exits yet for the project. See also #3704 and ​https://lists.rtems.org/pipermail/devel/2019-February/024988.html.

[Jens Schweikhardt]

If I had to devise a (rough?) guideline:

1) only pointer parameters are annotated (since scalars are [in] by language definition)
2) [in] indicates that the pointer must point to an initialized object (it may be dereferenced by the directive)
3) [out] indicates that the object pointed to may be written by the directive
4) [inout] If both 2) and 3) apply.

This is basically how I interpret these annotations from the Interface Definition Languages from a past life where I used CORBA with a C binding. Word-smithing suggestions welcome.