From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Jyri Sarha <jsarha@ti.com>
Cc: kernel-janitors@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
dri-devel@lists.freedesktop.org,
Julia Lawall <julia.lawall@lip6.fr>,
Tomi Valkeinen <tomi.valkeinen@ti.com>
Subject: Re: GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection
Date: Thu, 22 Sep 2016 20:38:41 +0200 [thread overview]
Message-ID: <d9b20442-3f84-82bd-d600-e3045a21da99@users.sourceforge.net> (raw)
In-Reply-To: <e9b443c7-2947-46d5-462f-58c175b774bc@ti.com>
>> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-09-22 18:38 UTC|newest]
Thread overview: 107+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <566ABCD9.1060404@users.sourceforge.net>
2016-08-18 19:42 ` [PATCH 0/2] GPU-DRM-Savage: Fine-tuning for savage_bci_cmdbuf() SF Markus Elfring
2016-08-18 19:45 ` [PATCH 1/2] GPU-DRM-Savage: Use memdup_user() rather than duplicating SF Markus Elfring
2016-08-18 19:48 ` [PATCH 2/2] GPU-DRM-Savage: Less function calls in savage_bci_cmdbuf() after error detection SF Markus Elfring
2016-08-19 7:50 ` Daniel Vetter
2016-10-12 12:04 ` SF Markus Elfring
2016-08-19 7:41 ` [PATCH 0/2] GPU-DRM-Savage: Fine-tuning for savage_bci_cmdbuf() Daniel Vetter
2016-09-18 16:48 ` [PATCH 0/5] drm/amdgpu: Fine-tuning for several function implementations SF Markus Elfring
2016-09-18 16:50 ` [PATCH 1/5] drm/amdgpu: Use kmalloc_array() in amdgpu_debugfs_gca_config_read() SF Markus Elfring
2016-09-19 17:25 ` Alex Deucher
2016-09-18 16:51 ` [PATCH 2/5] drm/amdgpu: Improve determination of sizes in two functions SF Markus Elfring
2016-09-18 16:52 ` [PATCH 3/5] drm/amdgpu: Rename a jump label in amdgpu_debugfs_regs_read() SF Markus Elfring
2016-09-18 16:53 ` [PATCH 4/5] drm/amdgpu: Rename a jump label in amdgpu_device_init() SF Markus Elfring
2016-09-19 13:56 ` Deucher, Alexander
2016-09-18 16:54 ` [PATCH 5/5] drm/amdgpu: Adjust checks for null pointers in nine functions SF Markus Elfring
2024-01-05 18:15 ` [PATCH 0/5] drm/amdgpu: Fine-tuning for several function implementations Markus Elfring
2024-01-05 18:29 ` Alex Deucher
2016-09-19 15:51 ` [PATCH 0/5] GPU-DRM: Fine-tuning for four " SF Markus Elfring
2016-09-19 15:53 ` [PATCH 1/5] GPU-DRM: Use kmalloc_array() in drm_legacy_addbufs_pci() SF Markus Elfring
2016-09-19 15:54 ` [PATCH 2/5] GPU-DRM: Replace two kzalloc() calls by kcalloc() " SF Markus Elfring
2016-09-19 15:55 ` [PATCH 3/5] GPU-DRM: Replace a kzalloc() call by kcalloc() in drm_legacy_addbufs_agp() SF Markus Elfring
2016-09-19 15:56 ` [PATCH 4/5] GPU-DRM: Replace a kzalloc() call by kcalloc() in drm_legacy_addbufs_sg() SF Markus Elfring
2016-09-21 11:22 ` Daniel Vetter
2016-09-19 15:58 ` [PATCH 5/5] GPU-DRM: Rename a jump label in drm_legacy_mapbufs() SF Markus Elfring
2016-09-20 8:54 ` [PATCH 0/6] GPU-DRM-GMA500: Fine-tuning for two function implementations SF Markus Elfring
2016-09-20 8:55 ` [PATCH 1/6] GPU-DRM-GMA500: Use kmalloc_array() in mid_get_vbt_data_r10() SF Markus Elfring
2016-09-20 10:06 ` Jani Nikula
2016-09-20 10:30 ` SF Markus Elfring
2016-09-20 8:57 ` [PATCH 2/6] GPU-DRM-GMA500: Rename a jump label " SF Markus Elfring
2016-09-20 10:05 ` Jani Nikula
2016-09-20 8:58 ` [PATCH 3/6] GPU-DRM-GMA500: Move a variable assignment " SF Markus Elfring
2016-09-20 8:59 ` [PATCH 4/6] GPU-DRM-GMA500: Fix indentation for a function call parameter " SF Markus Elfring
2016-09-20 9:00 ` [PATCH 5/6] GPU-DRM-GMA500: One error message less for a GCT revision mismatch in mid_get_vbt_data() SF Markus Elfring
2016-09-20 10:07 ` Jani Nikula
2016-09-20 10:32 ` SF Markus Elfring
2016-09-20 10:48 ` [PATCH 5/6] " Dan Carpenter
2016-09-20 11:03 ` SF Markus Elfring
2016-09-20 11:17 ` Dan Carpenter
2016-09-20 11:30 ` SF Markus Elfring
2016-09-20 12:08 ` [PATCH 5/6] " Jani Nikula
2016-09-20 20:23 ` Patrik Jakobsson
2016-09-20 9:01 ` [PATCH 6/6] GPU-DRM-GMA500: Rename a jump label " SF Markus Elfring
2016-09-20 10:08 ` Jani Nikula
2016-09-20 12:40 ` Dan Carpenter
2016-09-21 16:35 ` [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations SF Markus Elfring
2016-09-21 16:38 ` [PATCH 01/14] GPU-DRM-OMAP: Use kmalloc_array() in tiler_map_show() SF Markus Elfring
2016-09-21 16:39 ` [PATCH 02/14] GPU-DRM-OMAP: Replace another kmalloc() call by " SF Markus Elfring
2016-09-21 16:40 ` [PATCH 03/14] GPU-DRM-OMAP: Less function calls in tiler_map_show() after error detection SF Markus Elfring
2016-09-21 16:41 ` [PATCH 04/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation in tiler_map_show() Markus Elfring
2016-09-21 16:45 ` SF Markus Elfring
2016-09-21 16:46 ` [PATCH 05/14] GPU-DRM-OMAP: Improve a size determination in dmm_txn_append() SF Markus Elfring
2016-09-21 16:47 ` [PATCH 06/14] GPU-DRM-OMAP: Improve a size determination in omap_dmm_probe() SF Markus Elfring
2016-09-21 16:48 ` [PATCH 07/14] GPU-DRM-OMAP: Rename a jump label " SF Markus Elfring
2016-09-21 16:49 ` [PATCH 08/14] GPU-DRM-OMAP: Rename a jump label in dmm_txn_commit() SF Markus Elfring
2016-09-21 16:50 ` [PATCH 09/14] GPU-DRM-OMAP: Delete an unnecessary variable initialisation " SF Markus Elfring
2016-09-21 16:52 ` [PATCH 10/14] GPU-DRM-OMAP: Use kmalloc_array() in omap_gem_attach_pages() SF Markus Elfring
2016-09-21 16:53 ` [PATCH 11/14] GPU-DRM-OMAP: Replace a kzalloc() call by kcalloc() " SF Markus Elfring
2016-09-21 16:54 ` [PATCH 12/14] GPU-DRM-OMAP: Move a variable assignment " SF Markus Elfring
2016-09-21 16:55 ` [PATCH 13/14] GPU-DRM-OMAP: Rename a jump label in omap_gem_new_dmabuf() SF Markus Elfring
2016-09-21 16:56 ` [PATCH 14/14] GPU-DRM-OMAP: Rename a jump label in four functions SF Markus Elfring
2016-09-22 6:45 ` [PATCH 00/14] GPU-DRM-OMAP: Fine-tuning for several function implementations Daniel Vetter
2016-09-22 6:54 ` Laurent Pinchart
2016-09-22 9:11 ` SF Markus Elfring
2016-09-22 8:30 ` [PATCH 0/4] GPU-DRM-TILCDC: Fine-tuning for two " SF Markus Elfring
2016-09-22 8:31 ` [PATCH 1/4] GPU-DRM-TILCDC: Use kmalloc_array() in kfree_table_init() SF Markus Elfring
2016-09-22 16:55 ` Jyri Sarha
2016-09-22 8:32 ` [PATCH 2/4] GPU-DRM-TILCDC: Return directly after a failed kfree_table_init() in tilcdc_convert_slave_node() SF Markus Elfring
2016-09-22 10:58 ` Dan Carpenter
2016-09-22 16:57 ` Jyri Sarha
2016-09-22 18:17 ` SF Markus Elfring
2016-09-22 8:33 ` [PATCH 3/4] GPU-DRM-TILCDC: Less function calls in tilcdc_convert_slave_node() after error detection SF Markus Elfring
2016-09-22 17:04 ` Jyri Sarha
2016-09-22 18:38 ` SF Markus Elfring [this message]
2016-09-22 20:22 ` Jyri Sarha
2016-09-23 7:36 ` SF Markus Elfring
2016-09-23 10:37 ` Jyri Sarha
2016-09-23 10:55 ` SF Markus Elfring
2016-09-23 10:58 ` Rob Clark
2016-09-23 11:19 ` SF Markus Elfring
2016-09-23 11:31 ` Rob Clark
2016-09-23 12:17 ` SF Markus Elfring
2016-09-23 13:04 ` Rob Clark
2016-09-22 8:34 ` [PATCH 4/4] GPU-DRM-TILCDC: Delete unnecessary variable initialisations in tilcdc_convert_slave_node() SF Markus Elfring
2016-09-22 17:32 ` [PATCH 00/14] GPU-DRM-TTM: Fine-tuning for several function implementations SF Markus Elfring
2016-09-22 17:33 ` [PATCH 01/14] GPU-DRM-TTM: Use kmalloc_array() in two functions SF Markus Elfring
2016-09-22 17:34 ` [PATCH 02/14] GPU-DRM-TTM: Rename a jump label in ttm_alloc_new_pages() SF Markus Elfring
2016-09-22 17:35 ` [PATCH 03/14] GPU-DRM-TTM: Rename jump labels in ttm_page_pool_free() SF Markus Elfring
2016-09-22 17:36 ` [PATCH 04/14] GPU-DRM-TTM: Rename a jump label in ttm_page_pool_get_pages() SF Markus Elfring
2016-09-22 17:37 ` [PATCH 05/14] GPU-DRM-TTM: Use kmalloc_array() in two more functions SF Markus Elfring
2016-09-22 17:38 ` [PATCH 06/14] GPU-DRM-TTM: Rename a jump label in ttm_dma_pool_alloc_new_pages() SF Markus Elfring
2016-09-22 17:39 ` [PATCH 07/14] GPU-DRM-TTM: Rename jump labels in ttm_dma_page_pool_free() SF Markus Elfring
2016-09-22 17:40 ` [PATCH 08/14] GPU-DRM-TTM: Rename a jump label in ttm_dma_pool_shrink_scan() SF Markus Elfring
2016-09-22 17:41 ` [PATCH 09/14] GPU-DRM-TTM: Return directly after a failed kzalloc() in ttm_dma_page_alloc_init() SF Markus Elfring
2016-09-22 17:42 ` [PATCH 10/14] GPU-DRM-TTM: Return directly after a failed kobject_init_and_add() " SF Markus Elfring
2016-09-22 17:43 ` [PATCH 11/14] GPU-DRM-TTM: Return an error code only as a constant in ttm_dma_pool_init() SF Markus Elfring
2016-09-22 17:44 ` [PATCH 12/14] GPU-DRM-TTM: Less function calls in ttm_dma_pool_init() after error detection SF Markus Elfring
2016-09-22 17:45 ` [PATCH 13/14] GPU-DRM-TTM: Delete unnecessary variable initialisations in ttm_dma_pool_init() SF Markus Elfring
2016-09-22 17:46 ` [PATCH 14/14] GPU-DRM-TTM: Mark an array of text strings as "const" " SF Markus Elfring
2016-09-22 18:49 ` Joe Perches
2016-09-23 9:44 ` [PATCH 00/14] GPU-DRM-TTM: Fine-tuning for several function implementations Christian König
2016-09-23 10:20 ` SF Markus Elfring
2016-09-23 10:38 ` Christian König
2016-09-23 11:07 ` SF Markus Elfring
2016-09-23 11:17 ` Christian König
2016-09-23 11:49 ` SF Markus Elfring
2016-09-23 13:06 ` Christian König
2016-09-23 12:55 ` Dan Carpenter
2016-09-23 13:46 ` SF Markus Elfring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d9b20442-3f84-82bd-d600-e3045a21da99@users.sourceforge.net \
--to=elfring@users.sourceforge.net \
--cc=dri-devel@lists.freedesktop.org \
--cc=jsarha@ti.com \
--cc=julia.lawall@lip6.fr \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tomi.valkeinen@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).