| Message ID | 20181226171538.21984-1-philmd@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | cutils: Cleanup, improve documentation | expand |
Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-philmd@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20181226171538.21984-1-philmd@redhat.com
Subject: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5ac365d util/cutils: Move function documentations to the header
1d4b496 util/cutils: Move ctype macros to "cutils.h"
70db713 util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"
=== OUTPUT BEGIN ===
Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"...
WARNING: Block comments use a leading /* on a separate line
#42: FILE: include/qemu/cutils.h:160:
+/**
total: 0 errors, 1 warnings, 48 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/3: util/cutils: Move ctype macros to "cutils.h"...
WARNING: Block comments use a leading /* on a separate line
#100: FILE: include/qemu/cutils.h:6:
+/** unsigned ctype macros:
ERROR: code indent should never use tabs
#108: FILE: include/qemu/cutils.h:14:
+#define qemu_isalnum(c)^I^Iisalnum((unsigned char)(c))$
ERROR: code indent should never use tabs
#109: FILE: include/qemu/cutils.h:15:
+#define qemu_isalpha(c)^I^Iisalpha((unsigned char)(c))$
ERROR: code indent should never use tabs
#110: FILE: include/qemu/cutils.h:16:
+#define qemu_iscntrl(c)^I^Iiscntrl((unsigned char)(c))$
ERROR: code indent should never use tabs
#111: FILE: include/qemu/cutils.h:17:
+#define qemu_isdigit(c)^I^Iisdigit((unsigned char)(c))$
ERROR: code indent should never use tabs
#112: FILE: include/qemu/cutils.h:18:
+#define qemu_isgraph(c)^I^Iisgraph((unsigned char)(c))$
ERROR: code indent should never use tabs
#113: FILE: include/qemu/cutils.h:19:
+#define qemu_islower(c)^I^Iislower((unsigned char)(c))$
ERROR: code indent should never use tabs
#114: FILE: include/qemu/cutils.h:20:
+#define qemu_isprint(c)^I^Iisprint((unsigned char)(c))$
ERROR: code indent should never use tabs
#115: FILE: include/qemu/cutils.h:21:
+#define qemu_ispunct(c)^I^Iispunct((unsigned char)(c))$
ERROR: code indent should never use tabs
#116: FILE: include/qemu/cutils.h:22:
+#define qemu_isspace(c)^I^Iisspace((unsigned char)(c))$
ERROR: code indent should never use tabs
#117: FILE: include/qemu/cutils.h:23:
+#define qemu_isupper(c)^I^Iisupper((unsigned char)(c))$
ERROR: code indent should never use tabs
#118: FILE: include/qemu/cutils.h:24:
+#define qemu_isxdigit(c)^Iisxdigit((unsigned char)(c))$
ERROR: code indent should never use tabs
#119: FILE: include/qemu/cutils.h:25:
+#define qemu_tolower(c)^I^Itolower((unsigned char)(c))$
ERROR: code indent should never use tabs
#120: FILE: include/qemu/cutils.h:26:
+#define qemu_toupper(c)^I^Itoupper((unsigned char)(c))$
ERROR: code indent should never use tabs
#121: FILE: include/qemu/cutils.h:27:
+#define qemu_isascii(c)^I^Iisascii((unsigned char)(c))$
ERROR: code indent should never use tabs
#122: FILE: include/qemu/cutils.h:28:
+#define qemu_toascii(c)^I^Itoascii((unsigned char)(c))$
total: 15 errors, 1 warnings, 126 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/3: util/cutils: Move function documentations to the header...
WARNING: Block comments use a leading /* on a separate line
#73: FILE: include/qemu/cutils.h:156:
+/**
WARNING: Block comments use a leading /* on a separate line
#95: FILE: include/qemu/cutils.h:179:
+/**
WARNING: Block comments use a leading /* on a separate line
#124: FILE: include/qemu/cutils.h:208:
+/**
WARNING: Block comments use a leading /* on a separate line
#154: FILE: include/qemu/cutils.h:238:
+/**
WARNING: Block comments use a leading /* on a separate line
#183: FILE: include/qemu/cutils.h:267:
+/**
WARNING: Block comments use a leading /* on a separate line
#214: FILE: include/qemu/cutils.h:298:
+/**
WARNING: Block comments use a leading /* on a separate line
#225: FILE: include/qemu/cutils.h:309:
+/**
WARNING: Block comments use a leading /* on a separate line
#235: FILE: include/qemu/cutils.h:319:
+/**
WARNING: Block comments use a leading /* on a separate line
#263: FILE: include/qemu/cutils.h:347:
+/**
WARNING: Block comments use a leading /* on a separate line
#273: FILE: include/qemu/cutils.h:357:
+/**
WARNING: Block comments use a leading /* on a separate line
#304: FILE: include/qemu/cutils.h:388:
+/**
total: 0 errors, 11 warnings, 544 lines checked
Your patch 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/20181226171538.21984-1-philmd@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On 1/2/19 6:41 PM, no-reply@patchew.org wrote: > Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-philmd@redhat.com/ [...]> === OUTPUT BEGIN === > Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"... > WARNING: Block comments use a leading /* on a separate line > #42: FILE: include/qemu/cutils.h:160: > +/** I believe this warning is incorrect, since we use the /** marking for Doxygen generated documentation. The offending comment is: /** * size_to_str: * * Return human readable string for size @val. * Use IEC binary units like KiB, MiB, and so forth. * * @val: The value to format. * Can be anything that uint64_t allows (no more than "16 EiB"). * * Caller is responsible for passing it to g_free(). */ char *size_to_str(uint64_t val); Am I missing something? > Checking PATCH 2/3: util/cutils: Move ctype macros to "cutils.h"... > WARNING: Block comments use a leading /* on a separate line > #100: FILE: include/qemu/cutils.h:6: > +/** unsigned ctype macros: > > ERROR: code indent should never use tabs > #108: FILE: include/qemu/cutils.h:14: > +#define qemu_isalnum(c)^I^Iisalnum((unsigned char)(c))$ OK, this is code movement I forgot to fix :/ > Checking PATCH 3/3: util/cutils: Move function documentations to the header... > WARNING: Block comments use a leading /* on a separate line > #73: FILE: include/qemu/cutils.h:156: > +/** > > WARNING: Block comments use a leading /* on a separate line > #95: FILE: include/qemu/cutils.h:179: > +/** > > WARNING: Block comments use a leading /* on a separate line > #124: FILE: include/qemu/cutils.h:208: > +/** > > WARNING: Block comments use a leading /* on a separate line > #154: FILE: include/qemu/cutils.h:238: > +/** > > WARNING: Block comments use a leading /* on a separate line > #183: FILE: include/qemu/cutils.h:267: > +/** > > WARNING: Block comments use a leading /* on a separate line > #214: FILE: include/qemu/cutils.h:298: > +/** > > WARNING: Block comments use a leading /* on a separate line > #225: FILE: include/qemu/cutils.h:309: > +/** > > WARNING: Block comments use a leading /* on a separate line > #235: FILE: include/qemu/cutils.h:319: > +/** > > WARNING: Block comments use a leading /* on a separate line > #263: FILE: include/qemu/cutils.h:347: > +/** > > WARNING: Block comments use a leading /* on a separate line > #273: FILE: include/qemu/cutils.h:357: > +/** > > WARNING: Block comments use a leading /* on a separate line > #304: FILE: include/qemu/cutils.h:388: > +/**
Cc'ing Markus and Thomas who reviewed commit 8c06fbdf36b. On Thu, Jan 3, 2019 at 10:04 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 1/2/19 6:41 PM, no-reply@patchew.org wrote: > > Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-philmd@redhat.com/ > [...]> === OUTPUT BEGIN === > > Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"... > > WARNING: Block comments use a leading /* on a separate line > > #42: FILE: include/qemu/cutils.h:160: > > +/** > > I believe this warning is incorrect, since we use the /** marking for > Doxygen generated documentation. The offending comment is: > > /** > * size_to_str: > * > * Return human readable string for size @val. > * Use IEC binary units like KiB, MiB, and so forth. > * > * @val: The value to format. > * Can be anything that uint64_t allows (no more than "16 EiB"). > * > * Caller is responsible for passing it to g_free(). > */ > char *size_to_str(uint64_t val); > > Am I missing something? I had a quick look at scripts/checkpatch.pl: # Block comment styles # Block comments use /* on a line of its own if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/ $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank WARN("Block comments use a leading /* on a separate line\n" . $herecurr); } I am confused because the comment says it allow blank /**, which is the case here.