| Message ID | 20181103015821.30074-1-lbloch@janustech.com |
|---|---|
| State | New |
| Headers | show |
| Series | include: Add a comment to explain the origin of sizes' lookup table | expand |
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20181103015821.30074-1-lbloch@janustech.com
Subject: [Qemu-devel] [PATCH] include: Add a comment to explain the origin of sizes' lookup table
=== 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'
ed3386770c include: Add a comment to explain the origin of sizes' lookup table
=== OUTPUT BEGIN ===
Checking PATCH 1/1: include: Add a comment to explain the origin of sizes' lookup table...
ERROR: code indent should never use tabs
#42: FILE: include/qemu/units.h:27:
+ * ^Isuffix="KMGTPE";$
ERROR: code indent should never use tabs
#43: FILE: include/qemu/units.h:28:
+ * ^Ifor(i=10; i<64; i++) {$
ERROR: code indent should never use tabs
#44: FILE: include/qemu/units.h:29:
+ * ^I^Ival=2**i;$
ERROR: code indent should never use tabs
#45: FILE: include/qemu/units.h:30:
+ * ^I^Is=substr(suffix, int(i/10), 1);$
ERROR: code indent should never use tabs
#46: FILE: include/qemu/units.h:31:
+ * ^I^In=2**(i%10);$
ERROR: code indent should never use tabs
#47: FILE: include/qemu/units.h:32:
+ * ^I^Ipad=21-int(log(n)/log(10));$
ERROR: code indent should never use tabs
#48: FILE: include/qemu/units.h:33:
+ * ^I^Iprintf("#define S_%d%siB %*d\n", n, s, pad, val);$
ERROR: code indent should never use tabs
#49: FILE: include/qemu/units.h:34:
+ * ^I}$
total: 8 errors, 0 warnings, 24 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
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On 11/2/18 8:58 PM, Leonid Bloch wrote: > The lookup table for power-of-two sizes was added in commit 540b8492618eb > for the purpose of having convenient shortcuts for these sizes in cases > when the literal number has to be present at compile time, and > expressions as '(1 * KiB)' can not be used. One such case is the > stringification of sizes. Beyond that, it is convenient to use these > shortcuts for all power-of-two sizes, even if they don't have to be > literal numbers. > > Despite its convenience, this table introduced 55 lines of "dumb" code, > the purpose and origin of which are obscure without reading the message > of the commit which introduced it. This patch fixes that by adding a > comment to the code itself with a brief explanation for the reasoning > behind this table. This comment includes the short AWK script that > generated the table, so that anyone who's interested could make sure > that the values in it are correct (otherwise these values look as if > they were typed manually). > > Signed-off-by: Leonid Bloch <lbloch@janustech.com> > --- > include/qemu/units.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) I'm still not completely sold that we can't come up with a more elegant runtime solution that avoids the need for hard-coding stringified defaults at compile time; but if we keep these S_* constants, this comment definitely helps. Reviewed-by: Eric Blake <eblake@redhat.com>
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20181103015821.30074-1-lbloch@janustech.com
Subject: [Qemu-devel] [PATCH] include: Add a comment to explain the origin of sizes' lookup table
=== 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'
4b786d4cd6 include: Add a comment to explain the origin of sizes' lookup table
=== OUTPUT BEGIN ===
Checking PATCH 1/1: include: Add a comment to explain the origin of sizes' lookup table...
ERROR: code indent should never use tabs
#43: FILE: include/qemu/units.h:27:
+ * ^Isuffix="KMGTPE";$
ERROR: code indent should never use tabs
#44: FILE: include/qemu/units.h:28:
+ * ^Ifor(i=10; i<64; i++) {$
ERROR: code indent should never use tabs
#45: FILE: include/qemu/units.h:29:
+ * ^I^Ival=2**i;$
ERROR: code indent should never use tabs
#46: FILE: include/qemu/units.h:30:
+ * ^I^Is=substr(suffix, int(i/10), 1);$
ERROR: code indent should never use tabs
#47: FILE: include/qemu/units.h:31:
+ * ^I^In=2**(i%10);$
ERROR: code indent should never use tabs
#48: FILE: include/qemu/units.h:32:
+ * ^I^Ipad=21-int(log(n)/log(10));$
ERROR: code indent should never use tabs
#49: FILE: include/qemu/units.h:33:
+ * ^I^Iprintf("#define S_%d%siB %*d\n", n, s, pad, val);$
ERROR: code indent should never use tabs
#50: FILE: include/qemu/units.h:34:
+ * ^I}$
total: 8 errors, 0 warnings, 24 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
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
diff --git a/include/qemu/units.h b/include/qemu/units.h index 68a7758650..051c274ca2 100644 --- a/include/qemu/units.h +++ b/include/qemu/units.h @@ -17,6 +17,24 @@ #define PiB (INT64_C(1) << 50) #define EiB (INT64_C(1) << 60) +/* + * The following lookup table is intended to be used when a literal string of + * the number of bytes is required (for example if it needs to be stringified). + * It can also be used for generic shortcuts of power-of-two sizes. + * This table is generated using the AWK script below: + * + * BEGIN { + * suffix="KMGTPE"; + * for(i=10; i<64; i++) { + * val=2**i; + * s=substr(suffix, int(i/10), 1); + * n=2**(i%10); + * pad=21-int(log(n)/log(10)); + * printf("#define S_%d%siB %*d\n", n, s, pad, val); + * } + * } + */ + #define S_1KiB 1024 #define S_2KiB 2048 #define S_4KiB 4096
The lookup table for power-of-two sizes was added in commit 540b8492618eb for the purpose of having convenient shortcuts for these sizes in cases when the literal number has to be present at compile time, and expressions as '(1 * KiB)' can not be used. One such case is the stringification of sizes. Beyond that, it is convenient to use these shortcuts for all power-of-two sizes, even if they don't have to be literal numbers. Despite its convenience, this table introduced 55 lines of "dumb" code, the purpose and origin of which are obscure without reading the message of the commit which introduced it. This patch fixes that by adding a comment to the code itself with a brief explanation for the reasoning behind this table. This comment includes the short AWK script that generated the table, so that anyone who's interested could make sure that the values in it are correct (otherwise these values look as if they were typed manually). Signed-off-by: Leonid Bloch <lbloch@janustech.com> --- include/qemu/units.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)