Re: [PATCH v13 06/30] powerpc/ptrace: Adapt gpr32_get, gpr32_set functions for transaction

[Michael Ellerman <mpe@ellerman.id.au>]

Daniel Axtens <dja@axtens.net> writes:

> [ Unknown signature status ]
> Hi all,
>
> This is causing cppcheck warnings (having just landed in next):
>
> [arch/powerpc/kernel/ptrace.c:2062]: (error) Uninitialized variable: ckpt_regs
> [arch/powerpc/kernel/ptrace.c:2130]: (error) Uninitialized variable: ckpt_regs

Sigh.

> This is from...
>> -static int gpr32_get(struct task_struct *target,
>> +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)
>> +			    void *kbuf, void __user *ubuf, bool tm_active)
>>  {
>>  	const unsigned long *regs = &target->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;
>>  
>> -	if (target->thread.regs == NULL)
>> -		return -EIO;
>> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>> +	ckpt_regs = &target->thread.ckpt_regs.gpr[0];
>> +#endif
>> +	if (tm_active) {
>> +		regs = ckpt_regs;
> ... this bit here. If the ifdef doesn't trigger, cppcheck can't find an
> initialisation for ckpt_regs, so it complains.
>
> Techinically it's a false positive as (I assume!) tm_active cannot ever
> be true in the absense of CONFIG_PPC_TRANSACTIONAL_MEM.

That's correct, so the code is safe. See the one call site (which passes an int!):

  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
  static int tm_cgpr32_get(struct task_struct *target,
  		     const struct user_regset *regset,
  		     unsigned int pos, unsigned int count,
  		     void *kbuf, void __user *ubuf)
  {
  	return gpr32_get_common(target, regset, pos, count, kbuf, ubuf, 1);
  }

> Is there a nice simple fix we could deploy to squash this warning, or
> will we just live with it?

This series has been nothing but pain. Given we're already at v13, and people
really want this support to go in, I'm going to leave it in the tree.

Once it's in we can refactor the implementation, which is a bit of a mess, and
hopefully in the process fix the warnings.

cheers