On Mon, 1 Feb 2021 09:23:18 -0800 Sean Christopherson wrote:

On Mon, Feb 01, 2021, Dave Hansen wrote:

quoted

On 1/31/21 9:40 PM, Kai Huang wrote:

quoted

quoted

quoted

-	ret = sgx_drv_init();
+	/* Success if the native *or* virtual EPC driver initialized cleanly. */
+	ret = !!sgx_drv_init() & !!sgx_vepc_init();

If would create more dumb code and just add

ret = sgx_vepc_init()
if (ret)
        goto err_kthread;

Jarkko, I'm not sure I understand this suggestion.

quoted

Do you mean you want below?

	ret = sgx_drv_init();
	ret = sgx_vepc_init();
	if (ret)
		goto err_kthread;

This was Sean's original code, but Dave didn't like it.

The problem is it's wrong.  That snippet would incorrectly bail if drv_init()
succeeds but vepc_init() fails.

The alternative to the bitwise AND is to snapshot the result in two separate
variables:

	ret = sgx_drv_init();
	ret2 = sgx_vepc_init();
	if (ret && ret2)
		goto err_kthread;

or check the return from drv_init() _after_ vepc_init():

	ret = sgx_drv_init();
	if (sgx_vepc_init() && ret)
		goto err_kthread;


As evidenced by this thread, the behavior is subtle and easy to get wrong.  I
deliberately chose the option that was the weirdest specifically to reduce the
probability of someone incorrectly "cleaning up" the code.

quoted

Are you sure?  I remember the !!&!! abomination being Sean's doing. :)

Yep!  That 100% functionally correct horror is my doing.

quoted

quoted

Sean/Dave,

Please let me know which way you prefer.

Kai, I don't really know you are saying here.  In the end,
sgx_vepc_init() has to run regardless of whether sgx_drv_init() is
successful or not.  Also, we only want to 'goto err_kthraed' if *BOTH*
fail.  The code you have above will, for instance, 'goto err_kthread' if
sgx_drv_init() succeeds but sgx_vepc_init() fails.  It entirely
disregards the sgx_drv_init() error code.

Hi Dave, Sean,

Yeah sorry my bad. The example I provided won't work. So I'd like keep the !!
&!! :)

Thanks. 

Jarkko, please let us know if you still have concern.