Re: [v2] powerpc/ptrace: Fix cppcheck issue in gpr32_set_common/gpr32_get_common()

[Michael Ellerman]

On Sun, 2016-11-09 at 13:44:13 UTC, Simon Guo wrote:
> From: Simon Guo 
> 
> The ckpt_regs usage in gpr32_set_common/gpr32_get_common() will lead to
> following cppcheck error at ifndef CONFIG_PPC_TRANSACTIONAL_MEM case:
> 
> [arch/powerpc/kernel/ptrace.c:2062]:
> (error) Uninitialized variable: ckpt_regs
> [arch/powerpc/kernel/ptrace.c:2130]:
> (error) Uninitialized variable: ckpt_regs
> 
> The problem is due to gpr32_set_common() used ckpt_regs variable which
> only makes sense at #ifdef CONFIG_PPC_TRANSACTIONAL_MEM.
> 
> This patch fix this issue by passing in "regs" parameter instead.
> 
> Reported-by: Daniel Axtens 
> Signed-off-by: Simon Guo 

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/261831160d4df6bafe2f0e12e6

cheers

[PATCH v2] powerpc/ptrace: Fix cppcheck issue in gpr32_set_common/gpr32_get_common()

[wei . guo . simon]

From: Simon Guo 

The ckpt_regs usage in gpr32_set_common/gpr32_get_common() will lead to
following cppcheck error at ifndef CONFIG_PPC_TRANSACTIONAL_MEM case:

[arch/powerpc/kernel/ptrace.c:2062]:
(error) Uninitialized variable: ckpt_regs
[arch/powerpc/kernel/ptrace.c:2130]:
(error) Uninitialized variable: ckpt_regs

The problem is due to gpr32_set_common() used ckpt_regs variable which
only makes sense at #ifdef CONFIG_PPC_TRANSACTIONAL_MEM.

This patch fix this issue by passing in "regs" parameter instead.

Reported-by: Daniel Axtens 
Signed-off-by: Simon Guo 
---
 arch/powerpc/kernel/ptrace.c | 74 +---
 1 file changed, 29 insertions(+), 45 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index bf91658..74eadb4 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2065,33 +2065,12 @@ static const struct user_regset_view 
user_ppc_native_view = {
 static int gpr32_get_common(struct task_struct *target,
 const struct user_regset *regset,
 unsigned int pos, unsigned int count,
-   void *kbuf, void __user *ubuf, bool tm_active)
+   void *kbuf, void __user *ubuf,
+   unsigned long *regs)
 {
-   const unsigned long *regs = >thread.regs->gpr[0];
-   const unsigned long *ckpt_regs;
compat_ulong_t *k = kbuf;
compat_ulong_t __user *u = ubuf;
compat_ulong_t reg;
-   int i;
-
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-   ckpt_regs = >thread.ckpt_regs.gpr[0];
-#endif
-   if (tm_active) {
-   regs = ckpt_regs;
-   } else {
-   if (target->thread.regs == NULL)
-   return -EIO;
-
-   if (!FULL_REGS(target->thread.regs)) {
-   /*
-* We have a partial register set.
-* Fill 14-31 with bogus values.
-*/
-   for (i = 14; i < 32; i++)
-   target->thread.regs->gpr[i] = NV_REG_POISON;
-   }
-   }
 
pos /= sizeof(reg);
count /= sizeof(reg);
@@ -2133,29 +2112,13 @@ static int gpr32_get_common(struct task_struct *target,
 static int gpr32_set_common(struct task_struct *target,
 const struct user_regset *regset,
 unsigned int pos, unsigned int count,
-const void *kbuf, const void __user *ubuf, bool tm_active)
+const void *kbuf, const void __user *ubuf,
+unsigned long *regs)
 {
-   unsigned long *regs = >thread.regs->gpr[0];
-   unsigned long *ckpt_regs;
const compat_ulong_t *k = kbuf;
const compat_ulong_t __user *u = ubuf;
compat_ulong_t reg;
 
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-   ckpt_regs = >thread.ckpt_regs.gpr[0];
-#endif
-
-   if (tm_active) {
-   regs = ckpt_regs;
-   } else {
-   regs = >thread.regs->gpr[0];
-
-   if (target->thread.regs == NULL)
-   return -EIO;
-
-   CHECK_FULL_REGS(target->thread.regs);
-   }
-
pos /= sizeof(reg);
count /= sizeof(reg);
 
@@ -2220,7 +2183,8 @@ static int tm_cgpr32_get(struct task_struct *target,
 unsigned int pos, unsigned int count,
 void *kbuf, void __user *ubuf)
 {
-   return gpr32_get_common(target, regset, pos, count, kbuf, ubuf, 1);
+   return gpr32_get_common(target, regset, pos, count, kbuf, ubuf,
+   >thread.ckpt_regs.gpr[0]);
 }
 
 static int tm_cgpr32_set(struct task_struct *target,
@@ -2228,7 +2192,8 @@ static int tm_cgpr32_set(struct task_struct *target,
 unsigned int pos, unsigned int count,
 const void *kbuf, const void __user *ubuf)
 {
-   return gpr32_set_common(target, regset, pos, count, kbuf, ubuf, 1);
+   return gpr32_set_common(target, regset, pos, count, kbuf, ubuf,
+   >thread.ckpt_regs.gpr[0]);
 }
 #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
 
@@ -2237,7 +2202,21 @@ static int gpr32_get(struct task_struct *target,
 unsigned int pos, unsigned int count,
 void *kbuf, void __user *ubuf)
 {
-   return gpr32_get_common(target, regset, pos, count, kbuf, ubuf, 0);
+   int i;
+
+   if (target->thread.regs == NULL)
+   return -EIO;
+
+   if (!FULL_REGS(target->thread.regs)) {
+   /*
+* We have a partial register set.
+* Fill 14-31 with bogus values.
+*/
+   for (i = 14; i < 32; i++)
+   target->thread.regs->gpr[i] = NV_REG_POISON;
+   }
+   return gpr32_get_common(target, regset, pos, count, kbuf,