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 <rrich...@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.
