lkml.org 
[lkml]   [2019]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] EDAC, mc: Fix edac_mc_find() in case no device is found
On Tue, May 14, 2019 at 07:25:58AM +0000, Robert Richter wrote:
> The function should return NULL in case no device is found, but it
> always returns the last checked mc device from the list even if the
> index did not match. This patch fixes this.
>
> I did some analysis why this did not raise any issues for about 3
> years and the reason is that edac_mc_find() is mostly used to search
> for existing devices. Thus, the bug is not triggered.
>
> Fixes: c73e8833bec5 ("EDAC, mc: Fix locking around mc_devices list")
> Signed-off-by: Robert Richter <rrichter@marvell.com>
> ---
> drivers/edac/edac_mc.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index 13594ffadcb3..aeeaaf30b38a 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -688,10 +688,9 @@ struct mem_ctl_info *edac_mc_find(int idx)
> mci = list_entry(item, struct mem_ctl_info, link);
>
> if (mci->mc_idx >= idx) {
> - if (mci->mc_idx == idx) {
> - goto unlock;
> - }
> - break;
> + if (mci->mc_idx != idx)
> + mci = NULL;
> + goto unlock;
> }
> }

Can we simplify this silly code even more pls? I'm pasting the whole
function instead of a diff for clarity:

---
struct mem_ctl_info *edac_mc_find(int idx)
{
struct mem_ctl_info *mci = NULL;
struct list_head *item;

mutex_lock(&mem_ctls_mutex);

list_for_each(item, &mc_devices) {
mci = list_entry(item, struct mem_ctl_info, link);
if (mci->mc_idx == idx)
goto unlock;
}
mci = NULL;

unlock:
mutex_unlock(&mem_ctls_mutex);
return mci;
---
Thx.

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

\
 
 \ /
  Last update: 2019-05-14 11:43    [W:0.030 / U:8.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site