[Qemu-devel] [PATCH] target: mips: Add /* comments to fix checkpatch error

Jules Irenge posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190327103859.7736-1-jbi.octave@gmail.com
Maintainers: Aleksandar Markovic <amarkovic@wavecomp.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <arikalo@wavecomp.com>
target/mips/cp0_timer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] target: mips: Add /* comments to fix checkpatch error
Posted by Jules Irenge 5 years ago
Add /* comment  to fix checkpatch warning
"WARNING: Block comments use a leading /* on a separate line".

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 target/mips/cp0_timer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index ffa460be25..d6c9bb8169 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -42,8 +42,8 @@ uint32_t cpu_mips_get_random(CPUMIPSState *env)
 
     /* Don't return same value twice, so get another value */
     do {
-        /* Use a simple algorithm of Linear Congruential Generator
-         * from ISO/IEC 9899 standard. */
+        /* Use a simple algorithm of Linear Congruential Generator */
+        /* from ISO/IEC 9899 standard. */
         seed = 1103515245 * seed + 12345;
         idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
     } while (idx == prev_idx);
-- 
2.20.1


Re: [Qemu-devel] [PATCH] target: mips: Add /* comments to fix checkpatch error
Posted by Stefano Garzarella 5 years ago
On Wed, Mar 27, 2019 at 10:38:59AM +0000, Jules Irenge wrote:
> Add /* comment  to fix checkpatch warning
> "WARNING: Block comments use a leading /* on a separate line".
> 
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
>  target/mips/cp0_timer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
> index ffa460be25..d6c9bb8169 100644
> --- a/target/mips/cp0_timer.c
> +++ b/target/mips/cp0_timer.c
> @@ -42,8 +42,8 @@ uint32_t cpu_mips_get_random(CPUMIPSState *env)
>  
>      /* Don't return same value twice, so get another value */
>      do {
> -        /* Use a simple algorithm of Linear Congruential Generator
> -         * from ISO/IEC 9899 standard. */
> +        /* Use a simple algorithm of Linear Congruential Generator */
> +        /* from ISO/IEC 9899 standard. */

Thanks for this patch, but this is a multiline comment. Looking at
CODING_STYLE, section "7. Comment style", we should use this form:
    /*
     * like
     * this
     */

Thanks,
Stefano

Re: [Qemu-devel] [PATCH] target: mips: Add /* comments to fix checkpatch error
Posted by Aleksandar Markovic 5 years ago
> From: Stefano Garzarella <sgarzare@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH] target: mips: Add /* comments to fix checkpatch error
> 
> On Wed, Mar 27, 2019 at 10:38:59AM +0000, Jules Irenge wrote:
> > Add /* comment  to fix checkpatch warning
> > "WARNING: Block comments use a leading /* on a separate line".
> >
> > Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> > ---
> >  target/mips/cp0_timer.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
> > index ffa460be25..d6c9bb8169 100644
> > --- a/target/mips/cp0_timer.c
> > +++ b/target/mips/cp0_timer.c
> > @@ -42,8 +42,8 @@ uint32_t cpu_mips_get_random(CPUMIPSState *env)
> >
> >      /* Don't return same value twice, so get another value */
> >      do {
> > -        /* Use a simple algorithm of Linear Congruential Generator
> > -         * from ISO/IEC 9899 standard. */
> > +        /* Use a simple algorithm of Linear Congruential Generator */
> > +        /* from ISO/IEC 9899 standard. */
> 
> Thanks for this patch, but this is a multiline comment. Looking at
> CODING_STYLE, section "7. Comment style", we should use this form:
>     /*
>      * like
>      * this
>      */
> 
> Thanks,
> Stefano

Jules, Stefano is right. I would like to add that it would be nice to fix
another comment in the same file - the one that causes these warnings:

WARNING: Block comments use a leading /* on a separate line
#146: FILE: target/mips/cp0_timer.c:146:
+    /* ??? This callback should occur when the counter is exactly equal to

WARNING: Block comments use * on subsequent lines
#147: FILE: target/mips/cp0_timer.c:147:
+    /* ??? This callback should occur when the counter is exactly equal to
+       the comparator value.  Offset the count by one to avoid immediately

WARNING: Block comments use a trailing */ on a separate line
#148: FILE: target/mips/cp0_timer.c:148:
+       retriggering the callback before any virtual time has passed.  */

Regards,
Aleksandar
Re: [Qemu-devel] [PATCH] target: mips: Add /* comments to fix checkpatch error
Posted by Markus Armbruster 5 years ago
Jules Irenge <jbi.octave@gmail.com> writes:

> Add /* comment  to fix checkpatch warning
> "WARNING: Block comments use a leading /* on a separate line".
>
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
>  target/mips/cp0_timer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
> index ffa460be25..d6c9bb8169 100644
> --- a/target/mips/cp0_timer.c
> +++ b/target/mips/cp0_timer.c
> @@ -42,8 +42,8 @@ uint32_t cpu_mips_get_random(CPUMIPSState *env)
>  
>      /* Don't return same value twice, so get another value */
>      do {
> -        /* Use a simple algorithm of Linear Congruential Generator
> -         * from ISO/IEC 9899 standard. */
> +        /* Use a simple algorithm of Linear Congruential Generator */
> +        /* from ISO/IEC 9899 standard. */
>          seed = 1103515245 * seed + 12345;
>          idx = (seed >> 16) % nb_rand_tlb + env->CP0_Wired;
>      } while (idx == prev_idx);

Please do

           /*
            * Use a simple algorithm of Linear Congruential Generator
            * from ISO/IEC 9899 standard.
            */

instead.