From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034453AbcIVSjG (ORCPT ); Thu, 22 Sep 2016 14:39:06 -0400 Received: from mout.web.de ([212.227.15.14]:53325 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965609AbcIVSjD (ORCPT ); Thu, 22 Sep 2016 14:39:03 -0400 Subject: Re: GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection To: Jyri Sarha References: <566ABCD9.1060404@users.sourceforge.net> <2f3f7ad7-16a0-1dfb-d073-0d993cd767ee@users.sourceforge.net> <0be7fee0-64f7-fa02-0337-51376677343e@users.sourceforge.net> Cc: dri-devel@lists.freedesktop.org, David Airlie , Tomi Valkeinen , LKML , kernel-janitors@vger.kernel.org, Julia Lawall From: SF Markus Elfring Message-ID: Date: Thu, 22 Sep 2016 20:38:41 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:J2HiMSOyOC/QCLYp9p1fJlEJMfyy6brMrkf0MUupfZCDQuPEqz0 jcaa4viYf48338qrXtsZPZ8Eka+cTusI0LXAe9uIgBWBPUC5KHhqaemxRJXZjhGqrZ3kFiB cJjOYLraJ6iz7UVn2HWP6GNOu0bf0y/YndbU/PcF93Y9NvdVv+gEwtCGFIxz4CoZnMvByHY xD1g8r4r2jzz1uSulvUUQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:nvKK5PfGlww=:990k73MR3oQ41WCJnJv8Yv 1S8hSK7creofsZuVGmoSLOBFiHYFtij6Nr3JIQyjd5vnLS2jBHkGm78HzuGHNL5h4k2WDhIbA mGboRMYirexSdtAxBt1dL5ehxWj072k8Xt1eXMB/BSc5PjSEMwma8o881LQoQyPqIMfpyF08F VJpnP3E3T0lMqA6eFtRYHWyxa1x9MYFstTAaS2Tqy7FYve1ETYfCY0JPMsUSf2GNDor3ifPry 3Mp0oEJZGeOxhk/MbIJomirSXU1c7xQljNVn2KAL5Kwr+s6DimL8VQ2tQrJr0PToiP+OwqGfs fC6HY1DT4TGh0qQFrfcnADgaE0sW6AALyhBz7ZyRLIr05X8gu3o24L6BdJK+WLumiRZxvtVOy yQI9CvCZFaxYIQkoPDHglFdOj9xxp/6dNPxXtVPyDh1YabfDIPwbFKPx47n8/SphuxoS/pMyl KfsnmROMvDPLU3sduWnKvGcRG20OgwVLbWgM4QXCOTzG436XXf+SwiNoJRr/cysLxldnpfR3z ZWLDqCEXhRotr4rY1591JCqrW3Ol2HLZtpxASWxKW989/RoJcTI/avJPpvnFwUnmysyepO3I5 wJsBk5QJ8zpA/EGMskptSiLP4bBO51D1YWRT2MMELduV2GlMphTLvilqdpZhgRJOK6FJmXJgU qI1C65CkbAtzJ6PUukqz0yrBLx7O6UksuuLChwSl5fr0QOpzOKdCX1d17UIu2P6Cu76V7+myE XbSo1RyaFC/VnKqSdI5p81G8Slf9QTkIYKekhPwWDoamOTpReLZUtdKR5mjjY0OWk0cVeFW9k QX2ZuNb Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> The of_node_put() function was called in some cases >> by the tilcdc_convert_slave_node() function during error handling >> even if the passed variable contained a null pointer. >> >> * Adjust jump targets according to the Linux coding style convention. >> >> * Split a condition check for resource detection failures so that >> each pointer from these function calls will be checked immediately. >> >> See also background information: >> Topic "CWE-754: Improper check for unusual or exceptional conditions" >> Link: https://cwe.mitre.org/data/definitions/754.html >> > > I don't really agree with this patch. This kind of feedback can be fine at first glance. > There is no harm in calling of_node_put() with NULL as an argument The cost of additional function calls will be eventually not noticed just because they belong to an exception handling implementation so far. > and because of that there is no point in making the function more complex There is inherent software complexity involved. > and harder to maintain. How do you think about to discuss this aspect a bit more? I suggest to reconsider this design detail if it is really acceptable for the safe implementation of such a software module. * How much will it matter in general that one function call was performed in this use case without checking its return value immediately? * Should it usually be determined quicker if a required resource could be acquired before trying the next allocation? Regards, Markus