On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> This:
> 1) Replaces enums with C style defines.
Why?
> 2) Adds defines for some constants.
Definitions need to be referenced somewhere to explain their use. It's
not enough to add a constant, some text in the spec should mention that
constant. (The exception to this might be a group of constants where
individual constants don't need to be mentioned explicitly.)
> Signed-off-by: Arseny Krasnov <arseny.krasnov@xxxxxxxxxxxxx>
> ---
> virtio-vsock.tex | 42 ++++++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> index da7e641..62ab6b3 100644
> --- a/virtio-vsock.tex
> +++ b/virtio-vsock.tex
> @@ -86,23 +86,18 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
> operation constants:
>
> \begin{lstlisting}
> -enum {
> - VIRTIO_VSOCK_OP_INVALID = 0,
> -
> - /* Connect operations */
> - VIRTIO_VSOCK_OP_REQUEST = 1,
> - VIRTIO_VSOCK_OP_RESPONSE = 2,
> - VIRTIO_VSOCK_OP_RST = 3,
> - VIRTIO_VSOCK_OP_SHUTDOWN = 4,
> -
> - /* To send payload */
> - VIRTIO_VSOCK_OP_RW = 5,
> -
> - /* Tell the peer our credit info */
> - VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
> - /* Request the peer to send the credit info to us */
> - VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
> -};
> +#define VIRTIO_VSOCK_OP_INVALID 0
> +/* Connect operations */
> +#define VIRTIO_VSOCK_OP_REQUEST 1
> +#define VIRTIO_VSOCK_OP_RESPONSE 2
> +#define VIRTIO_VSOCK_OP_RST 3
> +#define VIRTIO_VSOCK_OP_SHUTDOWN 4
> +/* To send payload */
> +#define VIRTIO_VSOCK_OP_RW 5
> +/* Tell the peer our credit info */
> +#define VIRTIO_VSOCK_OP_CREDIT_UPDATE 6
> +/* Request the peer to send the credit info to us */
> +#define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
> \end{lstlisting}
>
> \subsubsection{Virtqueue Flow Control}\label{sec:Device Types / Socket Device / Device Operation / Virtqueue Flow Control}
> @@ -143,6 +138,10 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
> Currently only stream sockets are supported. \field{type} is 1 for stream
> socket types.
>
> +\begin{lstlisting}
> +#define VIRTIO_VSOCK_TYPE_STREAM 1
> +\end{lstlisting}
> +
> Stream sockets provide in-order, guaranteed, connection-oriented delivery
> without message boundaries.
>
> @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> hints are permanent once sent and successive packets with bits clear do not
> reset them.
>
> +\begin{lstlisting}
> +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT 1
> +\end{lstlisting}
The spec has no other _BIT constants. It uses bitmasks instead. I
suggest following that for consistency:
#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
#define VIRTIO_VSOCK_SHUTDOWN_SEND (1 << 1)
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
