[Libvir] proposal: remove contradictory indentation directive
Jim Meyering
jim at meyering.net
Wed Apr 9 18:42:01 UTC 2008
"Daniel P. Berrange" <berrange at redhat.com> wrote:
> On Wed, Apr 09, 2008 at 02:40:13PM +0100, Richard W.M. Jones wrote:
>> On Wed, Apr 09, 2008 at 02:43:22PM +0200, Jim Meyering wrote:
>> > Please don't add the "tab-width: 4" specifier.
>> > Specifying a tab-width at all in a new file with "indent-tabs-mode: nil"
>> > is a contradiction. The latter says there should be no TABs, yet
>> > the former says "when there are, give them width 4." Coding style
>> > guidelines are universal in their recommendations to stick with 8-byte
>> > TAB stops, independent of whether you actually use TAB or spaces.
>>
>> Agreed, good idea.
>
> ACK.
>
> Could we get a make syntax-check test to look for these bogus tabs too
I've done that.
Abbreviated patches below.
However, there are some "issues" to consider.
Any global space-changing delta like these is going to cause
trouble (conflicts) for people with pending changes and on branches.
This one isn't too bad (as these things go), since the TAB-to-space
change affects fewer than 1500 lines in 37 files. However, the
new rule enforces the coding standard only in files with an existing
"indent-tabs-mode: nil" directive. There are currently 42 .[ch] files
that don't have such a directive (excluding gnulib/).
$ git ls-files|grep -E '\.[ch]$'|xargs grep -L indent-tabs-mode|grep -v \
gnulib|wc -l
42
If I were to do the same for those remaining files,
the TAB-to-space change would modify an additional 2817 lines
in 28 files:
$ git ls-files|grep -E '\.[ch]$'|xargs grep -L indent-tabs-mode|grep -v \
gnulib|xargs grep -E '^ * '|wc -l
2817
$ git ls-files|grep -E '\.[ch]$'|xargs grep -L indent-tabs-mode|grep -v \
gnulib|xargs grep -El '^ * '|wc -l
28
My opinion is that if it's worth doing the first, it's also worth
finishing the job, ... but then I don't have any huge re-architecting
changes in my queue.
However, I don't want to overstate the case either.
The cost of a few space-change-provoked merges is pretty low
in the grand scheme of things.
What say you?
Convert 'em all?
-----------------------------------------------------------------
Here are the interesting parts of the two changes.
First, remove the 'tab-width: 4' directives and make sure they
stay gone:
remove all 'tab-width: 4' directives
Makefile.maint (sc_prohibit_tab_width_4): New rule.
Remove all of them, using this command:
git grep -l 'tab-width: 4' \
| xargs perl -ni -e '/^\s*\*\s*tab-width:\s*4\s*$/ or print'
Signed-off-by: Jim Meyering <meyering at redhat.com>
---
Makefile.maint | 9 +++++++++
proxy/libvirt_proxy.c | 1 -
...
tests/xmconfigtest.c | 1 -
78 files changed, 9 insertions(+), 77 deletions(-)
diff --git a/Makefile.maint b/Makefile.maint
index a357d74..fc21bf4 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -287,6 +287,15 @@ sc_trailing_blank:
{ echo '$(ME): found trailing blank(s)' \
1>&2; exit 1; } || :
+sp=[ ]
+# Some files used to containe a line like this: " * tab-width: 4",
+# contradicting an adjacent "indent-tabs-mode: nil" directive.
+# Make sure no more are introduced.
+sc_prohibit_tab_width_4:
+ @grep -E '^$(sp)*\*$(sp)*tab-width: 4$$' $$($(VC_LIST_EXCEPT)) && \
+ { echo '$(ME): found bogus "tab-width:" directive(s)' \
+ 1>&2; exit 1; } || :
+
# Match lines like the following, but where there is only one space
# between the options and the description:
# -D, --all-repeated[=delimit-method] print all duplicate lines\n
diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c
index bed3ae8..b28a3bc 100644
--- a/proxy/libvirt_proxy.c
+++ b/proxy/libvirt_proxy.c
@@ -857,6 +857,5 @@ int main(void) {
* indent-tabs-mode: nil
* c-indent-level: 4
* c-basic-offset: 4
- * tab-width: 4
* End:
*/
diff --git a/python/libvir.c b/python/libvir.c
index fe650e8..8ad1122 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -1464,6 +1464,5 @@ initcygvirtmod
* indent-tabs-mode: nil
* c-indent-level: 4
* c-basic-offset: 4
- * tab-width: 4
* End:
*/
...
---------------------------------------------------------------------
Then remove the offending in-indentation-only TABs from files
with "indent-tabs-mode: nil":
In a file with "indent-tabs-mode: nil", no TABs for indentation.
Convert offenders via "expand --initial".
* Makefile.maint (sc_TAB_in_indentation): New rule to prevent regression.
* proxy/libvirt_proxy.c: Convert indentation TABs to spaces.
* python/libvir.c, src/buf.c: src/conf.c, src/conf.h: Likewise.
* src/driver.h, src/internal.h, src/libvirt.c, src/lxc_conf.c: Likewise.
* src/openvz_driver.h, src/proxy_internal.c: Likewise.
* src/proxy_internal.h, src/qemu_conf.c, src/qemu_driver.c: Likewise.
* src/qparams.h, src/remote_internal.c, src/stats_linux.h: Likewise.
* src/util.c, src/virsh.c, src/virterror.c, src/xen_internal.c: Likewise.
* src/xen_unified.c, src/xen_unified.h, src/xend_internal.c: Likewise.
* src/xm_internal.c, src/xml.c, src/xs_internal.c: Likewise.
* tests/nodeinfotest.c, tests/sexpr2xmltest.c: Likewise.
Signed-off-by: Jim Meyering <meyering at redhat.com>
---
Makefile.maint | 9 +
proxy/libvirt_proxy.c | 652 ++++++++++++++++++++++++------------------------
python/libvir.c | 34 ++--
src/buf.c | 56 ++--
src/conf.c | 324 ++++++++++++------------
src/conf.h | 16 +-
src/driver.h | 260 ++++++++++----------
src/internal.h | 18 +-
src/libvirt.c | 174 +++++++-------
src/lxc_conf.c | 34 ++--
src/openvz_driver.h | 12 +-
src/proxy_internal.c | 244 +++++++++---------
src/proxy_internal.h | 26 +-
src/qemu_conf.c | 2 +-
src/qemu_driver.c | 10 +-
src/qparams.h | 2 +-
src/remote_internal.c | 8 +-
src/stats_linux.h | 6 +-
src/util.c | 8 +-
src/virsh.c | 66 +++---
src/virterror.c | 356 ++++++++++++++--------------
src/xen_internal.c | 212 ++++++++--------
src/xen_unified.c | 52 ++--
src/xen_unified.h | 74 +++---
src/xend_internal.c | 88 ++++----
src/xm_internal.c | 50 ++--
src/xml.c | 12 +-
src/xs_internal.c | 90 ++++----
tests/nodeinfotest.c | 2 +-
tests/sexpr2xmltest.c | 166 +++++++-------
30 files changed, 1536 insertions(+), 1527 deletions(-)
diff --git a/Makefile.maint b/Makefile.maint
index fc21bf4..a22c0f5 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -296,6 +296,15 @@ sc_prohibit_tab_width_4:
{ echo '$(ME): found bogus "tab-width:" directive(s)' \
1>&2; exit 1; } || :
+# Ensure that each file containing an "indent-tabs-mode: nil"
+# directive uses no TABs for indentation.
+sc_TAB_in_indentation:
+ @grep -lE '^ * ' /dev/null \
+ $$(grep -lE '^$(sp)*\*$(sp)*indent-tabs-mode: nil$$' \
+ $$($(VC_LIST_EXCEPT))) && \
+ { echo '$(ME): found TAB(s) used for indentation' \
+ 1>&2; exit 1; } || :
+
# Match lines like the following, but where there is only one space
# between the options and the description:
# -D, --all-repeated[=delimit-method] print all duplicate lines\n
diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c
index b28a3bc..6be3ad8 100644
--- a/proxy/libvirt_proxy.c
+++ b/proxy/libvirt_proxy.c
@@ -93,10 +93,10 @@ proxyInitXen(void) {
return(-1);
} else {
ret = xenHypervisorGetVersion(conn, &xenVersion);
- if (ret != 0) {
- fprintf(stderr, "Failed to get Xen hypervisor version\n");
- return(-1);
- }
+ if (ret != 0) {
+ fprintf(stderr, "Failed to get Xen hypervisor version\n");
+ return(-1);
+ }
}
ret = xenDaemonOpen_unix(conn, "/var/lib/xend/xend-socket");
if (ret < 0) {
@@ -110,12 +110,12 @@ proxyInitXen(void) {
}
ret = xenDaemonGetVersion(conn, &xenVersion2);
if (ret != 0) {
- fprintf(stderr, "Failed to get Xen daemon version\n");
- return(-1);
+ fprintf(stderr, "Failed to get Xen daemon version\n");
+ return(-1);
}
if (debug)
fprintf(stderr, "Connected to hypervisor %lu and daemon %lu\n",
- xenVersion, xenVersion2);
+ xenVersion, xenVersion2);
if (xenVersion2 > xenVersion)
xenVersion = xenVersion2;
return(0);
...
More information about the libvir-list
mailing list