RE: [bp:edac-cxl 2/5] drivers/edac/edac_device.c:705 edac_dev_register() error: uninitialized symbol 'ret'.

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Borislav Petkov <bp@alien8.de>
>Sent: 25 February 2025 14:54
>To: Dan Carpenter <dan.carpenter@linaro.org>; Shiju Jose
><shiju.jose@huawei.com>
>Cc: oe-kbuild@lists.linux.dev; lkp@intel.com; oe-kbuild-all@lists.linux.dev;
>Jonathan Cameron <jonathan.cameron@huawei.com>
>Subject: Re: [bp:edac-cxl 2/5] drivers/edac/edac_device.c:705
>edac_dev_register() error: uninitialized symbol 'ret'.
>
>On Tue, Feb 25, 2025 at 10:46:48AM +0300, Dan Carpenter wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git edac-cxl
>> head:   4fab25b62bc682c0df886d97dcb106d4989c9ff1
>> commit: e777719d5bec32af859990456f4b0e0e362f425a [2/5] EDAC: Add
>scrub
>> control feature
>> config: i386-randconfig-141-20250225
>> (https://download.01.org/0day-ci/archive/20250225/202502251009.0sGkolE
>> J-lkp@intel.com/config)
>> compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project
>> cd708029e0b2869e80abe31ddb175f7c35361f90)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new
>> version of the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> | Closes:
>> | https://lore.kernel.org/r/202502251009.0sGkolEJ-lkp@intel.com/
>>
>> smatch warnings:
>> drivers/edac/edac_device.c:705 edac_dev_register() error: uninitialized
>symbol 'ret'.
>
>Thanks for reporting.
>
>I went through the whole set and did some scrubbing of retvals. And that
>edac_dev_register() is already a mess. But cleaning that up is for another day.
>
>So, Shiju, I've pushed a edac-cxl2 branch, please run it again.

Thanks Borislav for fixing this error and changes looks good to me. 

Tested fine in the edac-cxl2 branch + following fix for the allmodconfig-x86_64-clang
build error reported in the other thread.

diff --git a/drivers/edac/mem_repair.c b/drivers/edac/mem_repair.c index cbe5a722715d..6f4a9dfcd7da 100755
--- a/drivers/edac/mem_repair.c
+++ b/drivers/edac/mem_repair.c
@@ -322,8 +322,8 @@ static int mem_repair_create_desc(struct device *dev,
                return -ENOMEM;
 
        for (i = 0; i < MR_MAX_ATTRS; i++) {
-               memcpy(&ctx->mem_repair_dev_attr[i].dev_attr,
-                      &dev_attr[i], sizeof(dev_attr[i]));
+               memcpy(&ctx->mem_repair_dev_attr[i], &dev_attr[i],
+                      sizeof(dev_attr[i]));
                ctx->mem_repair_attrs[i] =
                        &ctx->mem_repair_dev_attr[i].dev_attr.attr;
        }
  
>
>Thx.
>
>Full diff of the changes I've done:
>
>---
>diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c index
>081f77b1f125..16611515ab34 100644
>--- a/drivers/edac/edac_device.c
>+++ b/drivers/edac/edac_device.c
>@@ -581,7 +581,7 @@ static void edac_dev_release(struct device *dev)
> 	kfree(ctx);
> }
>
>-const struct device_type edac_dev_type = {
>+static const struct device_type edac_dev_type = {
> 	.name = "edac_dev",
> 	.release = edac_dev_release,
> };
>@@ -616,8 +616,9 @@ int edac_dev_register(struct device *parent, char
>*name,
> 	struct edac_dev_feat_ctx *ctx;
> 	int mem_repair_cnt = 0;
> 	int attr_gcnt = 0;
>+	int ret = -ENOMEM;
> 	int scrub_cnt = 0;
>-	int ret, feat;
>+	int feat;
>
> 	if (!parent || !name || !num_features || !ras_features)
> 		return -EINVAL;
>@@ -646,25 +647,19 @@ int edac_dev_register(struct device *parent, char
>*name,
> 		return -ENOMEM;
>
> 	ras_attr_groups = kcalloc(attr_gcnt + 1, sizeof(*ras_attr_groups),
>GFP_KERNEL);
>-	if (!ras_attr_groups) {
>-		ret = -ENOMEM;
>+	if (!ras_attr_groups)
> 		goto ctx_free;
>-	}
>
> 	if (scrub_cnt) {
> 		ctx->scrub = kcalloc(scrub_cnt, sizeof(*ctx->scrub),
>GFP_KERNEL);
>-		if (!ctx->scrub) {
>-			ret = -ENOMEM;
>+		if (!ctx->scrub)
> 			goto groups_free;
>-		}
> 	}
>
> 	if (mem_repair_cnt) {
> 		ctx->mem_repair = kcalloc(mem_repair_cnt, sizeof(*ctx-
>>mem_repair), GFP_KERNEL);
>-		if (!ctx->mem_repair) {
>-			ret = -ENOMEM;
>+		if (!ctx->mem_repair)
> 			goto data_mem_free;
>-		}
> 	}
>
> 	attr_gcnt = 0;
>@@ -673,9 +668,10 @@ int edac_dev_register(struct device *parent, char
>*name,
> 	for (feat = 0; feat < num_features; feat++, ras_features++) {
> 		switch (ras_features->ft_type) {
> 		case RAS_FEAT_SCRUB:
>-			if (!ras_features->scrub_ops ||
>-			    scrub_cnt != ras_features->instance)
>+			if (!ras_features->scrub_ops || scrub_cnt !=
>ras_features->instance) {
>+				ret = -EINVAL;
> 				goto data_mem_free;
>+			}
>
> 			dev_data = &ctx->scrub[scrub_cnt];
> 			dev_data->instance = scrub_cnt;
>@@ -690,8 +686,10 @@ int edac_dev_register(struct device *parent, char
>*name,
> 			attr_gcnt++;
> 			break;
> 		case RAS_FEAT_ECS:
>-			if (!ras_features->ecs_ops)
>+			if (!ras_features->ecs_ops) {
>+				ret = -EINVAL;
> 				goto data_mem_free;
>+			}
>
> 			dev_data = &ctx->ecs;
> 			dev_data->ecs_ops = ras_features->ecs_ops; @@ -705,8
>+703,10 @@ int edac_dev_register(struct device *parent, char *name,
> 			break;
> 		case RAS_FEAT_MEM_REPAIR:
> 			if (!ras_features->mem_repair_ops ||
>-			    mem_repair_cnt != ras_features->instance)
>+			    mem_repair_cnt != ras_features->instance) {
>+				ret = -EINVAL;
> 				goto data_mem_free;
>+			}
>
> 			dev_data = &ctx->mem_repair[mem_repair_cnt];
> 			dev_data->instance = mem_repair_cnt;
>
>--
>Regards/Gruss,
>    Boris.
>
>https://people.kernel.org/tglx/notes-about-netiquette

Thank,
Shiju