kexec: potetially using uninitialized variable

At the end of the function we check if "ret" has a negative error code, but it seems possible that it is uninitialized.

Fixes 12db5562e035 ('kexec: load and relocate purgatory at kernel load time')

Signed-off-by Dan Carpenter <[email protected]>

kernel/kexec_file.c +0 -0

index 503bc2d..63d1af3 100644

Hi Dan,

Quoted 11 lines · Dan Carpenter

At the end of the function we check if "ret" has a negative error code, but it seems possible that it is uninitialized.

Fixes 12db5562e035 ('kexec: load and relocate purgatory at kernel load time')

Signed-off-by Dan Carpenter <[email protected]>

kernel/kexec_file.c +0 -0

index 503bc2d..63d1af3 100644

kernel/kexec_file.c out: · line 801

Quoted 8 lines

kernel/kexec_file.c +1 -1

@@ -795,7 +795,7 @@ out:
 
 static int kexec_apply_relocations(struct kimage *image)
 {
-	int i, ret;
+	int i, ret = 0;
 	struct purgatory_info *pi = &image->purgatory_info;
 	Elf_Shdr *sechdrs = pi->sechdrs;
 

Look further, there is a condition at the beginning of the for loop:

        if (sechdrs[i].sh_type != SHT_RELA &&
            sechdrs[i].sh_type != SHT_REL)
            continue;

So, I think that's ok, but I don't konw if GCC is smart enough not to throw warnings.

Quoted 30 lines

Hi Dan,

Quoted 11 lines · Dan Carpenter

At the end of the function we check if "ret" has a negative error code, but it seems possible that it is uninitialized.

Fixes 12db5562e035 ('kexec: load and relocate purgatory at kernel load time')

Signed-off-by Dan Carpenter <[email protected]>

kernel/kexec_file.c +0 -0

index 503bc2d..63d1af3 100644

kernel/kexec_file.c out: · line 801

Quoted 8 lines

kernel/kexec_file.c +1 -1

@@ -795,7 +795,7 @@ out:
 
 static int kexec_apply_relocations(struct kimage *image)
 {
-	int i, ret;
+	int i, ret = 0;
 	struct purgatory_info *pi = &image->purgatory_info;
 	Elf_Shdr *sechdrs = pi->sechdrs;
 

Look further, there is a condition at the beginning of the for loop:

        if (sechdrs[i].sh_type != SHT_RELA &&
            sechdrs[i].sh_type != SHT_REL)
            continue;

Quoted 1 line

So, I think that's ok, but I don't konw if GCC is smart enough not to throw warnings.

Ah, right...

This wasn't a GCC warning. GCC misses a lot of uninitialized variable bugs so I'm doing this with Smatch.

Anyway, I'll patch this up in Smatch to not warn about this.

Am 11.03.2016 10:19, schrieb Dan Carpenter:

Quoted 35 lines · Xunlei Pang

Quoted 26 lines

Hi Dan,

Quoted 11 lines · Dan Carpenter

At the end of the function we check if "ret" has a negative error code, but it seems possible that it is uninitialized.

Fixes 12db5562e035 ('kexec: load and relocate purgatory at kernel load time')

Signed-off-by Dan Carpenter <[email protected]>

kernel/kexec_file.c +0 -0

index 503bc2d..63d1af3 100644

kernel/kexec_file.c out: · line 801

Quoted 8 lines

kernel/kexec_file.c +1 -1

@@ -795,7 +795,7 @@ out:
 
 static int kexec_apply_relocations(struct kimage *image)
 {
-	int i, ret;
+	int i, ret = 0;
 	struct purgatory_info *pi = &image->purgatory_info;
 	Elf_Shdr *sechdrs = pi->sechdrs;
 

Look further, there is a condition at the beginning of the for loop:

Quoted 5 lines

        if (sechdrs[i].sh_type != SHT_RELA &&
            sechdrs[i].sh_type != SHT_REL)
            continue;

So, I think that's ok, but I don't konw if GCC is smart enough not to throw warnings.

Ah, right...

Quoted 5 lines

This wasn't a GCC warning. GCC misses a lot of uninitialized variable bugs so I'm doing this with Smatch.

Anyway, I'll patch this up in Smatch to not warn about this.

I am not so sure about this. the point should be that the reviewer can read it easily not if gcc complains or not.

just my 2 cents,

re,
wh

I think we can modify the logic a bit to make code simple. Thus gcc will not complain about any more, and the logic is earier.

Quoted 2 lines · Minfei Huang

I think we can modify the logic a bit to make code simple. Thus gcc will not complain about any more, and the logic is earier.

This is a Smatch warning, not a GCC warning. If you think the new code is clearer, that's fine but don't just silence the warning to please Smatch. I'm pretty sure I can silence this warning in Smatch.

regards,
dan carpenter