From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.web.de ([212.227.15.3]:56828 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750821AbdKKJiV (ORCPT ); Sat, 11 Nov 2017 04:38:21 -0500 Subject: Re: KEYS: trusted: Use common error handling code in trusted_update() To: Julia Lawall , keyrings@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Cc: David Howells , James Morris , Mimi Zohar , "Serge E. Hallyn" , LKML , kernel-janitors@vger.kernel.org References: <479805df-edaf-1e9a-57be-d7c4f38e9d31@users.sourceforge.net> <658d88c1-b29b-cf8c-2ce0-8a2755ec9f33@users.sourceforge.net> From: SF Markus Elfring Message-ID: Date: Sat, 11 Nov 2017 10:37:36 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Sender: linux-integrity-owner@vger.kernel.org List-ID: > Safe means that cleanup code should appear once in a cascade at the end > of the function, to minimize the chance that anything will be overlooked. I find that the control flow of this function implementation does not fit to the mentioned ideal so far. > Moving the ret assignments to the end of the function and adding the > backward jumps doesn't make the code more understandable. Is this structure required if you would like to achieve something in the shown software design direction? > On the other hand, moving the kzalloc of new_p to the end of the function > could be helpful, Why do you think that the movement of this function call can finally work in the concrete software situation? > because it reduces the chance that new error handling code, > if any turns out to be needed, will forget this operation. Your expectation can be nice. > By why not just follow standard practice and free the structures in a > cascade in the inverse of the order in which they are allocated at the end > of the function? This is still happening here partly, isn't it? > There can be a descriptive label for each thing that needs to be freed. Which identifiers would you find more appropriate in comparison to my suggestion? * e_inval * e_nomem * free_data * free_payload Regards, Markus