Re: [PATCH v13 04/13] x86/sgx: Architectural structures

From: Jarkko Sakkinen
Date: Tue Aug 28 2018 - 04:08:23 EST


On Mon, Aug 27, 2018 at 12:41:29PM -0700, Dave Hansen wrote:
> > +/**
> > + * enum sgx_encls_leaves - return codes for ENCLS, ENCLU and ENCLV
> > + * %SGX_SUCCESS: No error.
> > + * %SGX_INVALID_SIG_STRUCT: SIGSTRUCT contains an invalid value.
> > + * %SGX_INVALID_ATTRIBUTE: Enclave is not attempting to access a resource
> > + * for which it is not authorized.
> > + * %SGX_BLKSTATE: EPC page is already blocked.
> > + * %SGX_INVALID_MEASUREMENT: SIGSTRUCT or EINITTOKEN contains an incorrect
> > + * measurement.
> ...
> > +enum sgx_return_codes {
> > + SGX_SUCCESS = 0,
> > + SGX_INVALID_SIG_STRUCT = 1,
> > + SGX_INVALID_ATTRIBUTE = 2,
> > + SGX_BLKSTATE = 3,
> > + SGX_INVALID_MEASUREMENT = 4,
> ...
>
> I don't think I've ever seen this particular method of commenting
> before. It's rather verbose and duplicates the names twice, which seems
> a bit silly.
>
> Can you talk a bit about why you chose to do it this way? I'd
> personally much rather see at least some brief comments inline with the
> definitions.

The reason that I chose this was

https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

It is recommended in the "kernel-doc for structs, unions, enums, and
typedefs" section.

/Jarkko