From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751655AbdKKJi0 (ORCPT ); Sat, 11 Nov 2017 04:38:26 -0500 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 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:zKpqtgATLf4C4Wp4p/KkwZP5AOWB2edn2mbu8c3554TeFbCA+lH TKFXD2GxW7XMXkctp+HHmLg48jhOmxs/PeZDg2drlxfr24reKwfpS9I/12x1D92xukaIQd3 3E8iOJK8mVgpd6jhJz9lMOTk7f/ICxZFGh6YybJbhkqr8efce/+4ITtLRlz49qSOPA9C0SE hgw0q0VYsbfl0S6/Oej9w== X-UI-Out-Filterresults: notjunk:1;V01:K0:UDct+RVuM1M=:TbdczDU+LT5OwNanUZe1b2 DjcZA0nRuKkbjMAdbttatDCFM8QSvdkwi58U2WcPv8tDVkbmPXCX7r5B+Er8dskf4APdsX/PP WaJaPD1izYO+WxmAsj7KoJg2E9sWAzJ6NQPnw6n49OE63uneepzWIVJHo3LdQnEa2uWo0Cg2q 9HZXWTw2Mdp+NpNN5t6BLQ6Z4NkpHDsV0r+hAzvoJ3SpSdiJDTq/0rXTwkVCzIecSGk3RVzvn c2EgpaAKUr1OJ48gDWp5t7+Kb2LjHGbdfemsIhsS2KTDRcKVaqi7yY+epz1yIdjvOEmw1YeTr iysiEgExNQKf8ag2LiotBiXwMjvHIyjSQpH497ZRK38ah5J0XdhPjCNjfn0NHutGLb/jyKv4Z MYdE3yc+cYwLP6PvI6SoZhkRn60OQpO2CNXM8LM50ale1pu7JziylgaZhCie0KmbvCtANPNHs 4z/9tyJWomGikp0xEfiF5m4VTqCpYnrSmbu3ekZIj8L0gcAh2K+Qx9a8yLQqGVrQwRbcHiCBn 1TdSY2exobhVFNKPEwodCA6i5t92mgj0oUj6tV8ATT/g3lRO3dLjbXdZ8/qHfK+LEibS/sHps 2nRkcG/wI89O2eODNOFM+u5XqgKJlkKtdiJAu766OX00WiTCoPWwfGUp58xWvZfETetdU8QXv jsu04ZTVSeRuv4hQ+PtuorZ7JRR4y277pHzeVli0a+oGKpQ9gXf1cptt0bOpIHfCiu96+tFEU 3giIlh+y/nqupC7Ajt0NxxLK+CVEz7GY0DwbnvZWP8F6Z0UH7OY/rRKrkTHV5UTIs0eixFjIu tStKEZFOOjxSjbLwaeTcr8VgyyIgxgDxJ0RlX09qtdNTD3C+Nw= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > 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