qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: qemu-devel@nongnu.org
Cc: aik@ozlabs.ru, qemu-ppc@nongnu.org, groug@kaod.org
Subject: Re: [PATCH qemu v14] spapr: Implement Open Firmware client interface
Date: Tue, 2 Mar 2021 13:17:40 +1100	[thread overview]
Message-ID: <YD2gRMjlkPP+qkHH@yekko.fritz.box> (raw)
In-Reply-To: <161414573477.13232.17399914296173612265@c667a6b167f6>

[-- Attachment #1: Type: text/plain, Size: 11143 bytes --]

On Tue, Feb 23, 2021 at 09:48:56PM -0800, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20210224054130.4540-1-aik@ozlabs.ru/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20210224054130.4540-1-aik@ozlabs.ru
> Subject: [PATCH qemu v14] spapr: Implement Open Firmware client interface
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]         patchew/20210224054130.4540-1-aik@ozlabs.ru -> patchew/20210224054130.4540-1-aik@ozlabs.ru
> Switched to a new branch 'test'
> 3fc539b spapr: Implement Open Firmware client interface
> 
> === OUTPUT BEGIN ===
> WARNING: line over 80 characters
> #268: FILE: hw/ppc/spapr.c:4463:
> +    ClientArchitectureSupportClass *casc = CLIENT_ARCHITECTURE_SUPPORT_CLASS(oc);

These style warnings in the qemu code proper will need to be fixed.

> WARNING: line over 80 characters
> #1431: FILE: hw/ppc/vof.h:29:
> +    INTERFACE_CHECK(ClientArchitectureSupport, (obj), TYPE_CLIENT_ARCHITECTURE_SUPPORT)
> 
> ERROR: code indent should never use tabs
> #1548: FILE: pc-bios/vof/bootmem.c:5:
> +^Iuint64_t kern[2];$

I'm a bit torn about these ones in the vof code.  I think it might be
simpler to go to non-tab indenting there (for .c, not .S) just to
avoid checkpatch whining all the time.

Or if you really don't want to update the coding style in VOF, it
would probably be good to include a patch altering checkpatch so it
excludes the VOF code (as it already does for the code imported into
linux-headers).

> 
> ERROR: code indent should never use tabs
> #1549: FILE: pc-bios/vof/bootmem.c:6:
> +^Iphandle chosen = ci_finddevice("/chosen");$
> 
> ERROR: code indent should never use tabs
> #1551: FILE: pc-bios/vof/bootmem.c:8:
> +^Iif (ci_getprop(chosen, "qemu,boot-kernel", kern, sizeof(kern)) !=$
> 
> ERROR: code indent should never use tabs
> #1552: FILE: pc-bios/vof/bootmem.c:9:
> +^I^I^Isizeof(kern))$
> 
> ERROR: code indent should never use tabs
> #1553: FILE: pc-bios/vof/bootmem.c:10:
> +^I^Ireturn;$
> 
> ERROR: code indent should never use tabs
> #1555: FILE: pc-bios/vof/bootmem.c:12:
> +^Ido_boot(kern[0], initrd, initrdsize);$
> 
> ERROR: externs should be avoided in .c files
> #1574: FILE: pc-bios/vof/ci.c:12:
> +extern uint32_t ci_entry(uint32_t params);
> 
> ERROR: externs should be avoided in .c files
> #1576: FILE: pc-bios/vof/ci.c:14:
> +extern unsigned long hv_rtas(unsigned long params);
> 
> ERROR: externs should be avoided in .c files
> #1577: FILE: pc-bios/vof/ci.c:15:
> +extern unsigned int hv_rtas_size;
> 
> ERROR: code indent should never use tabs
> #1581: FILE: pc-bios/vof/ci.c:19:
> +^Ivoid *rtasbase;$
> 
> ERROR: code indent should never use tabs
> #1582: FILE: pc-bios/vof/ci.c:20:
> +^Iuint32_t rtassize = 0;$
> 
> ERROR: code indent should never use tabs
> #1583: FILE: pc-bios/vof/ci.c:21:
> +^Iphandle rtas;$
> 
> ERROR: code indent should never use tabs
> #1585: FILE: pc-bios/vof/ci.c:23:
> +^Iif (strcmp("call-method", (void *)(unsigned long) pargs->service))$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1585: FILE: pc-bios/vof/ci.c:23:
> +       if (strcmp("call-method", (void *)(unsigned long) pargs->service))
> [...]
> 
> ERROR: code indent should never use tabs
> #1586: FILE: pc-bios/vof/ci.c:24:
> +^I^Ireturn false;$
> 
> ERROR: code indent should never use tabs
> #1588: FILE: pc-bios/vof/ci.c:26:
> +^Iif (strcmp("instantiate-rtas", (void *)(unsigned long) pargs->args[0]))$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1588: FILE: pc-bios/vof/ci.c:26:
> +       if (strcmp("instantiate-rtas", (void *)(unsigned long) pargs->args[0]))
> [...]
> 
> ERROR: code indent should never use tabs
> #1589: FILE: pc-bios/vof/ci.c:27:
> +^I^Ireturn false;$
> 
> ERROR: code indent should never use tabs
> #1591: FILE: pc-bios/vof/ci.c:29:
> +^Irtas = ci_finddevice("/rtas");$
> 
> ERROR: code indent should never use tabs
> #1592: FILE: pc-bios/vof/ci.c:30:
> +^Ici_getprop(rtas, "rtas-size", &rtassize, sizeof(rtassize));$
> 
> ERROR: code indent should never use tabs
> #1593: FILE: pc-bios/vof/ci.c:31:
> +^Iif (rtassize < hv_rtas_size)$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1593: FILE: pc-bios/vof/ci.c:31:
> +       if (rtassize < hv_rtas_size)
> [...]
> 
> ERROR: code indent should never use tabs
> #1594: FILE: pc-bios/vof/ci.c:32:
> +^I^Ireturn false;$
> 
> ERROR: code indent should never use tabs
> #1596: FILE: pc-bios/vof/ci.c:34:
> +^Irtasbase = (void *)(unsigned long) pargs->args[2];$
> 
> ERROR: code indent should never use tabs
> #1598: FILE: pc-bios/vof/ci.c:36:
> +^Imemcpy(rtasbase, hv_rtas, hv_rtas_size);$
> 
> ERROR: code indent should never use tabs
> #1599: FILE: pc-bios/vof/ci.c:37:
> +^Ipargs->args[pargs->nargs] = 0;$
> 
> ERROR: code indent should never use tabs
> #1600: FILE: pc-bios/vof/ci.c:38:
> +^Ipargs->args[pargs->nargs + 1] = pargs->args[2];$
> 
> ERROR: code indent should never use tabs
> #1602: FILE: pc-bios/vof/ci.c:40:
> +^Ireturn true;$
> 
> ERROR: code indent should never use tabs
> #1607: FILE: pc-bios/vof/ci.c:45:
> +^Iif (!prom_handle((void *)(unsigned long) args))$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1607: FILE: pc-bios/vof/ci.c:45:
> +       if (!prom_handle((void *)(unsigned long) args))
> [...]
> 
> ERROR: code indent should never use tabs
> #1608: FILE: pc-bios/vof/ci.c:46:
> +^I^Ici_entry(args);$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1622: FILE: pc-bios/vof/ci.c:60:
> +        for (i = 0; i < nargs; i++)
> [...]
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1626: FILE: pc-bios/vof/ci.c:64:
> +        for (i = 0; i < nret; i++)
> [...]
> 
> ERROR: spaces required around that '+' (ctx:VxV)
> #1627: FILE: pc-bios/vof/ci.c:65:
> +                args.args[nargs+i] = 0;
>                                 ^
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1629: FILE: pc-bios/vof/ci.c:67:
> +        if (ci_entry((uint32_t)(&args)) < 0)
> [...]
> 
> ERROR: code indent should never use tabs
> #1637: FILE: pc-bios/vof/ci.c:75:
> +^Icall_prom("exit", 0, 0);$
> 
> ERROR: code indent should never use tabs
> #1642: FILE: pc-bios/vof/ci.c:80:
> +^Ireturn call_prom("finddevice", 1, 1, path);$
> 
> ERROR: code indent should never use tabs
> #1647: FILE: pc-bios/vof/ci.c:85:
> +^Ireturn call_prom("getprop", 4, 1, ph, propname, prop, len);$
> 
> ERROR: code indent should never use tabs
> #1652: FILE: pc-bios/vof/ci.c:90:
> +^Ireturn call_prom("open", 1, 1, path);$
> 
> ERROR: code indent should never use tabs
> #1657: FILE: pc-bios/vof/ci.c:95:
> +^Icall_prom("close", 1, 0, ih);$
> 
> ERROR: code indent should never use tabs
> #1662: FILE: pc-bios/vof/ci.c:100:
> +^Iuint32_t ret = call_prom("claim", 3, 1, ADDR(virt), size, align);$
> 
> ERROR: code indent should never use tabs
> #1664: FILE: pc-bios/vof/ci.c:102:
> +^Ireturn (void *) (unsigned long) ret;$
> 
> ERROR: code indent should never use tabs
> #1669: FILE: pc-bios/vof/ci.c:107:
> +^Ireturn call_prom("release", 2, 1, ADDR(virt), size);$
> 
> ERROR: code indent should never use tabs
> #1792: FILE: pc-bios/vof/libc.c:5:
> +^Iint len = 0;$
> 
> ERROR: code indent should never use tabs
> #1794: FILE: pc-bios/vof/libc.c:7:
> +^Iwhile (*s != 0) {$
> 
> ERROR: code indent should never use tabs
> #1795: FILE: pc-bios/vof/libc.c:8:
> +^I^Ilen += 1;$
> 
> ERROR: code indent should never use tabs
> #1796: FILE: pc-bios/vof/libc.c:9:
> +^I^Is += 1;$
> 
> ERROR: code indent should never use tabs
> #1797: FILE: pc-bios/vof/libc.c:10:
> +^I}$
> 
> ERROR: code indent should never use tabs
> #1799: FILE: pc-bios/vof/libc.c:12:
> +^Ireturn len;$
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1805: FILE: pc-bios/vof/libc.c:18:
> +                if (*s1 != *s2)
> [...]
> 
> ERROR: braces {} are necessary for all arms of this statement
> #1833: FILE: pc-bios/vof/libc.c:46:
> +                if (*p1 != *p2)
> [...]
> 
> ERROR: return is not a function, parentheses are not required
> #1834: FILE: pc-bios/vof/libc.c:47:
> +                        return (*p1 - *p2);
> 
> ERROR: else should follow close brace '}'
> #1857: FILE: pc-bios/vof/libc.c:70:
> +        }
> +        else {
> 
> ERROR: code indent should never use tabs
> #1890: FILE: pc-bios/vof/main.c:6:
> +^Iregister unsigned long r3 __asm__("r3") = _r3;$
> 
> ERROR: code indent should never use tabs
> #1891: FILE: pc-bios/vof/main.c:7:
> +^Iregister unsigned long r4 __asm__("r4") = _r4;$
> 
> ERROR: code indent should never use tabs
> #1892: FILE: pc-bios/vof/main.c:8:
> +^Iregister unsigned long r5 __asm__("r5") = (unsigned long) _prom_entry;$
> 
> ERROR: code indent should never use tabs
> #1894: FILE: pc-bios/vof/main.c:10:
> +^I((client *)(uint32_t)addr)();$
> 
> ERROR: code indent should never use tabs
> #1899: FILE: pc-bios/vof/main.c:15:
> +^Iregister unsigned long r3 __asm__("r3");$
> 
> ERROR: code indent should never use tabs
> #1900: FILE: pc-bios/vof/main.c:16:
> +^Iregister unsigned long r4 __asm__("r4");$
> 
> ERROR: code indent should never use tabs
> #1901: FILE: pc-bios/vof/main.c:17:
> +^Iregister unsigned long r5 __asm__("r5");$
> 
> ERROR: code indent should never use tabs
> #1902: FILE: pc-bios/vof/main.c:18:
> +^Iuint64_t initrd = r3, initrdsize = r4;$
> 
> ERROR: code indent should never use tabs
> #1904: FILE: pc-bios/vof/main.c:20:
> +^Iboot_from_memory(initrd, initrdsize);$
> 
> ERROR: code indent should never use tabs
> #1905: FILE: pc-bios/vof/main.c:21:
> +^Ici_panic("*** No boot target ***\n");$
> 
> total: 63 errors, 2 warnings, 1738 lines checked
> 
> Commit 3fc539b07428 (spapr: Implement Open Firmware client interface) has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20210224054130.4540-1-aik@ozlabs.ru/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-03-02  2:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-24  5:41 [PATCH qemu v14] spapr: Implement Open Firmware client interface Alexey Kardashevskiy
2021-02-24  5:48 ` no-reply
2021-03-02  2:17   ` David Gibson [this message]
2021-03-02  3:35 ` David Gibson
2021-03-02  7:21   ` Alexey Kardashevskiy
2021-03-02  9:37     ` BALATON Zoltan
2021-03-09  5:33       ` David Gibson
2021-03-09  5:29     ` David Gibson
2021-03-09  7:28       ` Alexey Kardashevskiy
2021-03-09 14:00         ` BALATON Zoltan
2021-03-10  1:55           ` Alexey Kardashevskiy
2021-03-10  2:40             ` David Gibson
2021-03-10  4:52               ` Alexey Kardashevskiy
2021-03-10  4:11         ` David Gibson

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=YD2gRMjlkPP+qkHH@yekko.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=aik@ozlabs.ru \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).