All of lore.kernel.org
 help / color / mirror / Atom feed
From: walter harms <wharms@bfs.de>
To: Andrey Utkin <andrey.krieger.utkin@gmail.com>
Cc: davem@davemloft.net, sparclinux@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator
Date: Tue, 05 Aug 2014 07:11:52 +0000	[thread overview]
Message-ID: <53E083B8.4090008@bfs.de> (raw)
In-Reply-To: <1407185261-1799-1-git-send-email-andrey.krieger.utkin@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1620 bytes --]



Am 04.08.2014 22:47, schrieb Andrey Utkin:
> This commit is a guesswork, but it seems to make sense to drop this
> break, as otherwise the following line is never executed and becomes
> dead code. And that following line actually saves the result of
> local calculation by the pointer given in function argument. So the
> proposed change makes sense if this code in the whole makes sense (but I
> am unable to analyze it in the whole).
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id641
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
> ---
>  arch/sparc/math-emu/math_32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
> index aa4d55b..5ce8f2f 100644
> --- a/arch/sparc/math-emu/math_32.c
> +++ b/arch/sparc/math-emu/math_32.c
> @@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
>  		case 0: fsr = *pfsr;
>  			if (IR = -1) IR = 2;
>  			/* fcc is always fcc0 */

The patch looks ok, but can somebody comment on this comment ?
what "fcc" ? should it be a fsr ?


> -			fsr &= ~0xc00; fsr |= (IR << 10); break;
> +			fsr &= ~0xc00; fsr |= (IR << 10);

nitpicking:
   fsr &= ~0xc00;
   fsr |= (IR << 10);

It is better readable.

re,
 wh

>  			*pfsr = fsr;
>  			break;
>  		case 1: rd->s = IR; break;
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: walter harms <wharms@bfs.de>
To: Andrey Utkin <andrey.krieger.utkin@gmail.com>
Cc: davem@davemloft.net, sparclinux@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator
Date: Tue, 05 Aug 2014 07:11:52 +0000	[thread overview]
Message-ID: <53E083B8.4090008@bfs.de> (raw)
In-Reply-To: <1407185261-1799-1-git-send-email-andrey.krieger.utkin@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1615 bytes --]



Am 04.08.2014 22:47, schrieb Andrey Utkin:
> This commit is a guesswork, but it seems to make sense to drop this
> break, as otherwise the following line is never executed and becomes
> dead code. And that following line actually saves the result of
> local calculation by the pointer given in function argument. So the
> proposed change makes sense if this code in the whole makes sense (but I
> am unable to analyze it in the whole).
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id641
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
> ---
>  arch/sparc/math-emu/math_32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
> index aa4d55b..5ce8f2f 100644
> --- a/arch/sparc/math-emu/math_32.c
> +++ b/arch/sparc/math-emu/math_32.c
> @@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
>  		case 0: fsr = *pfsr;
>  			if (IR = -1) IR = 2;
>  			/* fcc is always fcc0 */

The patch looks ok, but can somebody comment on this comment ?
what "fcc" ? should it be a fsr ?


> -			fsr &= ~0xc00; fsr |= (IR << 10); break;
> +			fsr &= ~0xc00; fsr |= (IR << 10);

nitpicking:
   fsr &= ~0xc00;
   fsr |= (IR << 10);

It is better readable.

re,
 wh

>  			*pfsr = fsr;
>  			break;
>  		case 1: rd->s = IR; break;
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: walter harms <wharms@bfs.de>
To: Andrey Utkin <andrey.krieger.utkin@gmail.com>
Cc: davem@davemloft.net, sparclinux@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator
Date: Tue, 05 Aug 2014 09:11:52 +0200	[thread overview]
Message-ID: <53E083B8.4090008@bfs.de> (raw)
In-Reply-To: <1407185261-1799-1-git-send-email-andrey.krieger.utkin@gmail.com>



Am 04.08.2014 22:47, schrieb Andrey Utkin:
> This commit is a guesswork, but it seems to make sense to drop this
> break, as otherwise the following line is never executed and becomes
> dead code. And that following line actually saves the result of
> local calculation by the pointer given in function argument. So the
> proposed change makes sense if this code in the whole makes sense (but I
> am unable to analyze it in the whole).
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=81641
> Reported-by: David Binderman <dcb314@hotmail.com>
> Signed-off-by: Andrey Utkin <andrey.krieger.utkin@gmail.com>
> ---
>  arch/sparc/math-emu/math_32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/sparc/math-emu/math_32.c b/arch/sparc/math-emu/math_32.c
> index aa4d55b..5ce8f2f 100644
> --- a/arch/sparc/math-emu/math_32.c
> +++ b/arch/sparc/math-emu/math_32.c
> @@ -499,7 +499,7 @@ static int do_one_mathemu(u32 insn, unsigned long *pfsr, unsigned long *fregs)
>  		case 0: fsr = *pfsr;
>  			if (IR == -1) IR = 2;
>  			/* fcc is always fcc0 */

The patch looks ok, but can somebody comment on this comment ?
what "fcc" ? should it be a fsr ?


> -			fsr &= ~0xc00; fsr |= (IR << 10); break;
> +			fsr &= ~0xc00; fsr |= (IR << 10);

nitpicking:
   fsr &= ~0xc00;
   fsr |= (IR << 10);

It is better readable.

re,
 wh

>  			*pfsr = fsr;
>  			break;
>  		case 1: rd->s = IR; break;

  parent reply	other threads:[~2014-08-05  7:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-04 20:47 [PATCH] arch/sparc/math-emu/math_32.c: drop stray break operator Andrey Utkin
2014-08-04 20:47 ` Andrey Utkin
2014-08-04 20:47 ` Andrey Utkin
2014-08-05  3:32 ` David Miller
2014-08-05  3:32   ` David Miller
2014-08-05  3:32   ` David Miller
2014-08-05  7:11 ` walter harms [this message]
2014-08-05  7:11   ` walter harms
2014-08-05  7:11   ` walter harms
2014-08-05 19:49   ` David Miller
2014-08-05 19:49     ` David Miller
2014-08-05 19:49     ` David Miller
2014-08-06  7:09     ` walter harms
2014-08-06  7:09       ` walter harms
2014-08-06  7:09       ` walter harms

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53E083B8.4090008@bfs.de \
    --to=wharms@bfs.de \
    --cc=andrey.krieger.utkin@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.