12:31 p.m.
... and why they are (probably) not errors.
bindtextdomain
daemon/guestfsd.c
erlang/erl-guestfs-proto.c
examples/copy_over.c
examples/create_disk.c
examples/display_icon.c
examples/inspect_vm.c
examples/mount_local.c
examples/virt-dhcp-address.c
tests/c-api/test-add-drive-opts.c
tests/c-api/test-add-libvirt-dom.c
tests/c-api/test-command.c
tests/c-api/test-config.c
tests/c-api/test-create-handle.c
tests/c-api/test-debug-to-file.c
tests/c-api/test-just-header.c
tests/c-api/test-last-errno.c
tests/c-api/test-private-data.c
tests/c-api/test-user-cancel.c
tests/charsets/test-charset-fidelity.c
tests/mount-local/test-parallel-mount-local.c
tests/regressions/rhbz501893.c
tests/regressions/rhbz790721.c
maint.mk: the above files do not call bindtextdomain
make: *** [sc_bindtextdomain] Error 1
I think for examples, tests and the rather special Erlang case, it's
wrong to demand calls to bindtextdomain. The daemon is arguable, but
we don't include any locale data in the appliance.
cast_of_argument_to_free
generator/erlang.ml:403: pr " free ((char *) optargs_s.%s);\n" n
generator/ocaml.ml:588: pr " free ((char *) optargs_s.%s);\n" n
generator/python.ml:437: pr " free ((char **) optargs_s.%s);\n" n
maint.mk: don't cast free argument
make: *** [sc_cast_of_argument_to_free] Error 1
It seems to me this is another shortcoming of 'const' in C.
We currently define the optargs structs as having const char *
string fields, eg:
struct guestfs_add_domain_argv {
//...
const char *libvirturi;
//...
}
I tried to change this so that the fields are plain 'char *' but the
structs are const, but that doesn't work. ie. The following code
would not work:
some_func (const char *uri)
{
const struct guestfs_add_domain_argv optargs = { libvirturi = uri };
guestfs_add_domain_argv (g, dom, &optargs);
}
Apparently const-ness of a struct does not get inherited by fields
inside it, probably because const-ness of a struct just indicates the
pointer to the string is const, not the string itself. (Phantom types
solve this problem properly and much more generally - having a const
keyword in C is an unnecessary mistake IMHO).
Anyway, the cast seems necessary in the three cases above.
error_message_period
php/README-PHP:33: echo ("Error: ".guestfs_last_error ($g)."\n");
php/README-PHP:44: echo ("Error: ".guestfs_last_error ($g)."\n");
php/extension/guestfs_php_002.phpt:30: die ("Error: ".guestfs_last_error
($g)."\n");
php/extension/guestfs_php_002.phpt:35: echo ("Error: ".guestfs_last_error
($g)."\n");
po-docs/ja.po-37794-"force>)."
po-docs/libguestfs-docs.pot-33647-"I<--resize-force>)."
po-docs/uk.po-35901-"force>)."
resize/resize.ml:285: error (f_"%s: unknown partition table type\nvirt-resize
only supports MBR (DOS) and GPT partition tables.") infile
resize/resize.ml:685: error (f_"You cannot use --expand when there is no
surplus space to expand into. You need to make the target disk larger by at least
%s.")
resize/resize.ml:697: error (f_"You cannot use --shrink when there is no
deficit (see 'deficit' in the virt-resize(1) man page).");
resize/resize.ml:714: error (f_"There is a deficit of %Ld bytes (%s). You need
to make the target disk larger by at least this amount or adjust your resizing
requests.")
src/launch-appliance.c:758: error (g, _("command failed: %s\nerrno: %s\n\nIf qemu
is located on a non-standard path, try setting the LIBGUESTFS_QEMU\nenvironment variable.
There may also be errors printed above."),
src/launch-libvirt.c-841- "'--format' option, or via the
optional 'format' argument to 'add-drive'."),
src/launch.c-196- "This is a limitation of qemu."));
src/launch.c-284- "This is a limitation of qemu."));
src/libvirtdomain.c-571- "See ATTACHING TO RUNNING DAEMONS in
guestfs(3) for further information."));
maint.mk: found error message ending in period
make: *** [sc_error_message_period] Error 1
The PHP warning seems to be a bug in the script. The other cases are
acceptable because the error messages are proper sentences.
makefile_at_at_check
erlang/examples/Makefile.am:1248: --verbatim $(srcdir)/create_disk.erl:@EXAMPLE1@ \
erlang/examples/Makefile.am:1249: --verbatim $(srcdir)/inspect_vm.erl:@EXAMPLE2@ \
examples/Makefile.am:1375: --verbatim $(srcdir)/create_disk.c:@EXAMPLE1@ \
examples/Makefile.am:1376: --verbatim $(srcdir)/inspect_vm.c:@EXAMPLE2@ \
fish/Makefile.am:1611: --insert $(srcdir)/guestfish-actions.pod:@ACTIONS@ \
fish/Makefile.am:1612: --insert $(srcdir)/guestfish-commands.pod:@FISH_COMMANDS@ \
java/examples/Makefile.am:2570: --verbatim $(srcdir)/CreateDisk.java:@EXAMPLE1@ \
java/examples/Makefile.am:2571: --verbatim $(srcdir)/InspectVM.java:@EXAMPLE2@ \
ocaml/examples/Makefile.am:2848: --verbatim $(srcdir)/create_disk.ml:@EXAMPLE1@ \
ocaml/examples/Makefile.am:2849: --verbatim $(srcdir)/inspect_vm.ml:@EXAMPLE2@ \
perl/examples/Makefile.am:2985: --verbatim $(srcdir)/create_disk.pl:@EXAMPLE1@ \
perl/examples/Makefile.am:2986: --verbatim $(srcdir)/inspect_vm.pl:@EXAMPLE2@ \
po-docs/ja/Makefile.am:3215: --insert $(srcdir)/guestfs-actions.pod:@ACTIONS@ \
po-docs/ja/Makefile.am:3216: --insert
$(srcdir)/guestfs-availability.pod:@AVAILABILITY@ \
po-docs/ja/Makefile.am:3217: --insert $(srcdir)/guestfs-structs.pod:@STRUCTS@ \
po-docs/ja/Makefile.am:3225: --insert $(srcdir)/guestfish-actions.pod:@ACTIONS@ \
po-docs/ja/Makefile.am:3226: --insert $(srcdir)/guestfish-commands.pod:@FISH_COMMANDS@
\
po-docs/ja/Makefile.am:3234: --insert sysprep-extra-options.pod:@EXTRA_OPTIONS@
\
po-docs/ja/Makefile.am:3235: --insert sysprep-operations.pod:@OPERATIONS@ \
po-docs/uk/Makefile.am:3353: --insert $(srcdir)/guestfs-actions.pod:@ACTIONS@ \
po-docs/uk/Makefile.am:3354: --insert
$(srcdir)/guestfs-availability.pod:@AVAILABILITY@ \
po-docs/uk/Makefile.am:3355: --insert $(srcdir)/guestfs-structs.pod:@STRUCTS@ \
po-docs/uk/Makefile.am:3363: --insert $(srcdir)/guestfish-actions.pod:@ACTIONS@ \
po-docs/uk/Makefile.am:3364: --insert $(srcdir)/guestfish-commands.pod:@FISH_COMMANDS@
\
po-docs/uk/Makefile.am:3372: --insert sysprep-extra-options.pod:@EXTRA_OPTIONS@
\
po-docs/uk/Makefile.am:3373: --insert sysprep-operations.pod:@OPERATIONS@ \
python/examples/Makefile.am:3611: --verbatim $(srcdir)/create_disk.py:@EXAMPLE1@ \
python/examples/Makefile.am:3612: --verbatim $(srcdir)/inspect_vm.py:@EXAMPLE2@ \
ruby/examples/Makefile.am:3931: --verbatim $(srcdir)/create_disk.rb:@EXAMPLE1@ \
ruby/examples/Makefile.am:3932: --verbatim $(srcdir)/inspect_vm.rb:@EXAMPLE2@ \
src/Makefile.am:4293: --insert $(srcdir)/guestfs-actions.pod:@ACTIONS@ \
src/Makefile.am:4294: --insert $(srcdir)/guestfs-availability.pod:@AVAILABILITY@ \
src/Makefile.am:4295: --insert $(srcdir)/guestfs-structs.pod:@STRUCTS@ \
sysprep/Makefile.am:4460: --insert sysprep-extra-options.pod:@EXTRA_OPTIONS@ \
sysprep/Makefile.am:4461: --insert sysprep-operations.pod:@OPERATIONS@ \
maint.mk: use $(...), not @...@
make: *** [sc_makefile_at_at_check] Error 1
Real bug (in libguestfs). Podwrapper should be changed to use a
different character for its substitutions. There is a possibility
that these could be accidentally substituted by autoconf.
prohibit_always-defined_macros
cat/virt-ls.c:#define O_CLOEXEC 0
daemon/daemon.h:#define O_CLOEXEC 0
daemon/guestfsd.c:# define O_CLOEXEC 0
examples/mount_local.c:#define O_CLOEXEC 0
fish/fish.h:#define O_CLOEXEC 0
generator/tests_c_api.ml:#define O_CLOEXEC 0
src/guestfs-internal.h:#define O_CLOEXEC 0
src/info.c:#define O_CLOEXEC 0
tests/c-api/test-last-errno.c:#define O_CLOEXEC 0
tests/c-api/test-user-cancel.c:#define O_CLOEXEC 0
tests/xml/fake-libvirt-xml.c:#define O_CLOEXEC 0
maint.mk: define the above via some gnulib .h file
make: *** [sc_prohibit_always-defined_macros] Error 1
I don't think I understand what's wrong about this. Does gnulib
define O_CLOEXEC now?
prohibit_doubled_word
po/pl.po:2947:do do
maint.mk: doubled words
make: *** [sc_prohibit_doubled_word] Error 1
Apparently 'do do' is OK in Polish ...
prohibit_empty_lines_at_EOF
contrib/intro/overview.png
contrib/intro/tools.png
contrib/intro/virt-manager-t.png
contrib/intro/virt-manager.png
contrib/intro/vmm-icons-t.png
contrib/intro/vmm-icons.png
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch
erlang/tests/010-load.erl
guestfs-release-notes.txt
html/draft.png
logo/fish.png
po/libguestfs.pot
tests/data/bin-i586-dynamic
tests/data/bin-sparc-dynamic
tests/data/bin-win32.exe
tests/data/bin-win64.exe
tests/data/bin-x86_64-dynamic
tests/data/filesanddirs-100M.tar.xz
tests/data/filesanddirs-10M.tar.xz
tests/data/helloworld.tar
tests/data/helloworld.tar.gz
tests/data/helloworld.tar.xz
tests/data/known-4
tests/data/known-5
tests/data/lib-i586.so
tests/data/lib-sparc.so
tests/data/lib-win32.dll
tests/data/lib-win64.dll
tests/data/lib-x86_64.so
tests/data/mbr-ext2-empty.img.gz
tests/guests/guest-aux/debian-packages
tests/guests/guest-aux/minimal-hive
tests/guests/guest-aux/windows-software
tests/guests/guest-aux/windows-system
tests/regressions/rhbz576879.sh
maint.mk: empty line(s) or no newline at EOF
make: *** [sc_prohibit_empty_lines_at_EOF] Error 1
One problem with this test is it's matching binaries.
prohibit_magic_number_exit
po-docs/ja.po:59861:"might be called indirectly from L<exit(3)>, which can
cause unexpected "
po-docs/libguestfs-docs.pot:50902:"might be called indirectly from L<exit(3)>,
which can cause unexpected "
po-docs/uk.po:57345:"might be called indirectly from L<exit(3)>, which can
cause unexpected "
src/guestfs.pod:2028:callback might be called indirectly from L<exit(3)>, which can
cause
tests/mount-local/test-parallel-mount-local.c:104: exit (77);
tests/mount-local/test-parallel-mount-local.c:110: exit (77);
tests/regressions/rhbz790721.c:79: exit (77);
maint.mk: use EXIT_* values rather than magic number
make: *** [sc_prohibit_magic_number_exit] Error 1
I think exit (77) is acceptable. To automake, this means that the
test has been skipped, and there is no constant for it. However
writing a regexp that matches all numbers except "77" is quite hard.
prohibit_path_max_allocation
daemon/initrd.c:41: char filename[PATH_MAX];
daemon/initrd.c:126: char fullpath[PATH_MAX];
daemon/inotify.c:310: char buf[PATH_MAX];
daemon/link.c:38: char link[PATH_MAX];
daemon/link.c:63: char link[PATH_MAX];
daemon/realpath.c:86: char ret[PATH_MAX+1] = "/";
daemon/xattr.c:272: char pathname[PATH_MAX];
src/launch-appliance.c:179: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
src/launch-libvirt.c:200: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
src/launch-libvirt.c:224: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
src/launch-unix.c:64: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
maint.mk: Avoid stack allocations of size PATH_MAX
make: *** [sc_prohibit_path_max_allocation] Error 1
The daemon ones are bugs in libguestfs. I haven't fixed them yet.
However the use of UNIX_PATH_MAX looks OK to me. I think the regexp
is over-matching.
prohibit_strcmp
examples/mount_local.c:150: if (p && strcmp (p+1, "bash") == 0) {
examples/virt-dhcp-address.c:129: if (strcmp (guest_type, "linux") == 0) {
examples/virt-dhcp-address.c:134: if (strcmp (guest_distro, "fedora") == 0
||
examples/virt-dhcp-address.c:135: strcmp (guest_distro, "rhel") == 0 ||
examples/virt-dhcp-address.c:136: strcmp (guest_distro, "redhat-based")
== 0) {
examples/virt-dhcp-address.c:139: else if (strcmp (guest_distro, "debian")
== 0 ||
examples/virt-dhcp-address.c:140: strcmp (guest_distro, "ubuntu")
== 0) {
examples/virt-dhcp-address.c:151: else if (strcmp (guest_type, "windows") ==
0) {
test-tool/test-tool.c:43://#define STRNEQ(a,b) (strcmp((a),(b)) != 0)
maint.mk: maint.mk: replace strcmp calls above with STREQ/STRNEQ
make: *** [sc_prohibit_strcmp] Error 1
We want to use strcmp in examples. They don't follow the ordinary
code conventions used in the rest of libguestfs.
There's something wrong with check matching test-tool.c.
prohibit_strcmp_and_strncmp
examples/mount_local.c:150: if (p && strcmp (p+1, "bash") == 0) {
examples/virt-dhcp-address.c:129: if (strcmp (guest_type, "linux") == 0) {
examples/virt-dhcp-address.c:134: if (strcmp (guest_distro, "fedora") == 0
||
examples/virt-dhcp-address.c:135: strcmp (guest_distro, "rhel") == 0 ||
examples/virt-dhcp-address.c:136: strcmp (guest_distro, "redhat-based")
== 0) {
examples/virt-dhcp-address.c:139: else if (strcmp (guest_distro, "debian")
== 0 ||
examples/virt-dhcp-address.c:140: strcmp (guest_distro, "ubuntu")
== 0) {
examples/virt-dhcp-address.c:151: else if (strcmp (guest_type, "windows") ==
0) {
maint.mk: use STREQ(LEN) in place of the above uses of strcmp(strncmp)
make: *** [sc_prohibit_strcmp_and_strncmp] Error 1
Examples should be excluded.
prohibit_strncpy
src/launch-appliance.c:178: strncpy (addr.sun_path, guestfsd_sock, UNIX_PATH_MAX);
src/launch-libvirt.c:199: strncpy (addr.sun_path, guestfsd_sock, UNIX_PATH_MAX);
src/launch-libvirt.c:223: strncpy (addr.sun_path, console_sock, UNIX_PATH_MAX);
src/launch-unix.c:63: strncpy (addr.sun_path, sockpath, UNIX_PATH_MAX);
maint.mk: do not use strncpy, period
make: *** [sc_prohibit_strncpy] Error 1
I think use of strncpy is justified here.
prohibit_test_minus_ao
appliance/init:26:if [ ! -L /etc/init.d/udev -a -x /etc/init.d/udev ]; then
configure.ac:1276: [test "x$GOBJECT_LIBS" != "x" -a
"x$GIO_LIBS" != "x"])
edit/test-virt-edit.sh:32:if [ "$(../cat/virt-cat -a test.qcow2 /etc/test3)" !=
"a
edit/test-virt-edit.sh:47: if [ "$(../cat/virt-cat -a test.qcow2
/etc/test3)" != "1
edit/test-virt-edit.sh:61:if [ "$(../fish/guestfish -i -a test.qcow2 --ro lstat
/etc/test3 | grep -E '^(mode|uid|gid):' | sort)" != "gid: 11
fish/test-mount-local.sh:36:if [ $# -gt 0 -a "$1" = "--run-test" ];
then
format/test-virt-format.sh:29:if [ "$(../cat/virt-filesystems -a test1.img)" !=
"/dev/sda1" ]; then
fuse/test-fuse-umount-race.sh:75:if [ "$(../fish/guestfish -a test-copy.qcow2 --ro
-i is-file /test-umount)" != "true" ]; then
fuse/test-fuse.sh:55:if [ ! -x "$guestfish" -o ! -x "$guestmount" ];
then
src/api-support/update-from-tarballs.sh:39: if [ $v != "1.2.0" -a $v !=
"1.3.0" -a ! -f $v ]; then
sysprep/test-virt-sysprep-script.sh:40:if [ ! -f stamp-script1.sh -o ! -f
stamp-script2.sh ]; then
tests/md/test-inspect-fstab-md.sh:33:if [ -z "$f" -o ! -f "$f" ];
then
tools/test-virt-make-fs.sh:48:if [ "$ntfs3g_available" = "yes" -a
"$ntfsprogs_available" = "yes" ]; then
maint.mk: use "test C1 && test C2", not "test C1 -a C2"; use
"test C1 || test C2", not "test C1 -o C2"
make: *** [sc_prohibit_test_minus_ao] Error 1
What was wrong with -a and -o? Perhaps we should exempt this test for
scripts that explicitly begin #!/bin/bash, since presumably these will
use bash's built-in test.
prohibit_trailing_blank_lines
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch
guestfs-release-notes.txt
po/libguestfs.pot
tests/guests/guest-aux/debian-packages
maint.mk: found trailing blank line(s)
make: *** [sc_prohibit_trailing_blank_lines] Error 1
guestfs-release-notes.txt and po/libguestfs.pot are generated files.
debian-packages is effectively a binary file.
prohibit_undesirable_word_seq
BUGS:147:can not
guestfs-release-notes.pod:561:can not
guestfs-release-notes.pod:1630:can not
guestfs-release-notes.pod:1657:can not
guestfs-release-notes.pod:1665:can not
guestfs-release-notes.txt:495:can not
guestfs-release-notes.txt:1550:can not
guestfs-release-notes.txt:1577:can not
guestfs-release-notes.txt:1585:can not
po-docs/ja.po:31860:can not
po-docs/ja.po:34550:can not
po-docs/ja.po:34577:can not
po-docs/ja.po:34585:can not
po-docs/libguestfs-docs.pot:28130:can not
po-docs/libguestfs-docs.pot:30835:can not
po-docs/libguestfs-docs.pot:30871:can not
po-docs/libguestfs-docs.pot:30882:can not
po-docs/uk.po:30301:can not
po-docs/uk.po:32978:can not
po-docs/uk.po:33005:can not
po-docs/uk.po:33013:can not
maint.mk: undesirable word sequence
make: *** [sc_prohibit_undesirable_word_seq] Error 1
require_config_h
examples/copy_over.c
examples/create_disk.c
examples/display_icon.c
examples/inspect_vm.c
examples/mount_local.c
examples/virt-dhcp-address.c
tests/c-api/test-just-header.c
maint.mk: the above files do not include <config.h>
make: *** [sc_require_config_h] Error 1
Examples shouldn't include <config.h>. The test-just-header.c test
needs to not include it, because of what it is testing.
require_config_h_first
examples/copy_over.c
examples/create_disk.c
examples/display_icon.c
examples/inspect_vm.c
examples/mount_local.c
examples/virt-dhcp-address.c
tests/c-api/test-just-header.c
maint.mk: the above files include some other header before <config.h>
make: *** [sc_require_config_h_first] Error 1
trailing_blank
TODO:405: - swap devices (both of block device and file) should be wiped. This may
Binary file contrib/intro/overview.png matches
Binary file contrib/intro/tools.png matches
Binary file contrib/intro/virt-manager-t.png matches
Binary file contrib/intro/virt-manager.png matches
Binary file contrib/intro/vmm-icons-t.png matches
Binary file contrib/intro/vmm-icons.png matches
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:34:
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:58:
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:75:
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:86:
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:95:
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:101:
contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch:102:--
guestfs-release-notes.txt:300:
guestfs-release-notes.txt:303:
guestfs-release-notes.txt:305:
guestfs-release-notes.txt:308:
guestfs-release-notes.txt:313:
guestfs-release-notes.txt:316:
guestfs-release-notes.txt:319:
guestfs-release-notes.txt:322:
guestfs-release-notes.txt:325:
guestfs-release-notes.txt:327:
guestfs-release-notes.txt:330:
guestfs-release-notes.txt:332:
guestfs-release-notes.txt:335:
guestfs-release-notes.txt:337:
guestfs-release-notes.txt:344:
guestfs-release-notes.txt:346:
guestfs-release-notes.txt:348:
guestfs-release-notes.txt:356:
guestfs-release-notes.txt:360:
guestfs-release-notes.txt:364:
guestfs-release-notes.txt:367:
guestfs-release-notes.txt:374:
guestfs-release-notes.txt:376:
guestfs-release-notes.txt:419:
guestfs-release-notes.txt:422:
guestfs-release-notes.txt:425:
guestfs-release-notes.txt:428:
guestfs-release-notes.txt:431:
guestfs-release-notes.txt:434:
guestfs-release-notes.txt:436:
guestfs-release-notes.txt:438:
guestfs-release-notes.txt:441:
guestfs-release-notes.txt:444:
guestfs-release-notes.txt:446:
guestfs-release-notes.txt:449:
guestfs-release-notes.txt:451:
guestfs-release-notes.txt:453:
guestfs-release-notes.txt:455:
guestfs-release-notes.txt:457:
guestfs-release-notes.txt:459:
guestfs-release-notes.txt:461:
guestfs-release-notes.txt:464:
guestfs-release-notes.txt:466:
guestfs-release-notes.txt:468:
guestfs-release-notes.txt:470:
guestfs-release-notes.txt:473:
guestfs-release-notes.txt:475:
guestfs-release-notes.txt:477:
guestfs-release-notes.txt:479:
guestfs-release-notes.txt:481:
guestfs-release-notes.txt:483:
guestfs-release-notes.txt:536:
guestfs-release-notes.txt:540:
guestfs-release-notes.txt:542:
guestfs-release-notes.txt:545:
guestfs-release-notes.txt:547:
guestfs-release-notes.txt:556:
guestfs-release-notes.txt:558:
guestfs-release-notes.txt:560:
guestfs-release-notes.txt:562:
guestfs-release-notes.txt:564:
guestfs-release-notes.txt:571:
guestfs-release-notes.txt:578:
guestfs-release-notes.txt:584:
guestfs-release-notes.txt:587:
guestfs-release-notes.txt:589:
guestfs-release-notes.txt:592:
guestfs-release-notes.txt:595:
guestfs-release-notes.txt:601:
guestfs-release-notes.txt:623:
guestfs-release-notes.txt:626:
guestfs-release-notes.txt:629:
guestfs-release-notes.txt:631:
guestfs-release-notes.txt:634:
guestfs-release-notes.txt:637:
guestfs-release-notes.txt:639:
guestfs-release-notes.txt:642:
guestfs-release-notes.txt:644:
guestfs-release-notes.txt:647:
guestfs-release-notes.txt:671:
guestfs-release-notes.txt:674:
guestfs-release-notes.txt:677:
guestfs-release-notes.txt:684:
guestfs-release-notes.txt:686:
guestfs-release-notes.txt:711:
guestfs-release-notes.txt:715:
guestfs-release-notes.txt:718:
guestfs-release-notes.txt:732:
guestfs-release-notes.txt:735:
guestfs-release-notes.txt:737:
guestfs-release-notes.txt:743:
guestfs-release-notes.txt:750:
guestfs-release-notes.txt:752:
guestfs-release-notes.txt:754:
guestfs-release-notes.txt:765:
guestfs-release-notes.txt:778:
guestfs-release-notes.txt:780:
guestfs-release-notes.txt:788:
guestfs-release-notes.txt:790:
guestfs-release-notes.txt:793:
guestfs-release-notes.txt:795:
guestfs-release-notes.txt:797:
guestfs-release-notes.txt:800:
guestfs-release-notes.txt:804:
guestfs-release-notes.txt:841:
guestfs-release-notes.txt:843:
guestfs-release-notes.txt:846:
guestfs-release-notes.txt:849:
guestfs-release-notes.txt:855:
guestfs-release-notes.txt:857:
guestfs-release-notes.txt:860:
guestfs-release-notes.txt:867:
guestfs-release-notes.txt:877:
guestfs-release-notes.txt:879:
guestfs-release-notes.txt:881:
guestfs-release-notes.txt:885:
guestfs-release-notes.txt:892:
guestfs-release-notes.txt:894:
guestfs-release-notes.txt:896:
guestfs-release-notes.txt:898:
guestfs-release-notes.txt:900:
guestfs-release-notes.txt:903:
guestfs-release-notes.txt:905:
guestfs-release-notes.txt:908:
guestfs-release-notes.txt:910:
guestfs-release-notes.txt:913:
guestfs-release-notes.txt:932:
guestfs-release-notes.txt:935:
guestfs-release-notes.txt:937:
guestfs-release-notes.txt:939:
guestfs-release-notes.txt:941:
guestfs-release-notes.txt:943:
guestfs-release-notes.txt:945:
guestfs-release-notes.txt:947:
guestfs-release-notes.txt:972:
guestfs-release-notes.txt:975:
guestfs-release-notes.txt:977:
guestfs-release-notes.txt:979:
guestfs-release-notes.txt:982:
guestfs-release-notes.txt:984:
guestfs-release-notes.txt:987:
guestfs-release-notes.txt:989:
guestfs-release-notes.txt:993:
guestfs-release-notes.txt:998:
guestfs-release-notes.txt:1001:
guestfs-release-notes.txt:1004:
guestfs-release-notes.txt:1006:
guestfs-release-notes.txt:1009:
guestfs-release-notes.txt:1012:
guestfs-release-notes.txt:1016:
guestfs-release-notes.txt:1020:
guestfs-release-notes.txt:1022:
guestfs-release-notes.txt:1024:
guestfs-release-notes.txt:1026:
guestfs-release-notes.txt:1028:
guestfs-release-notes.txt:1030:
guestfs-release-notes.txt:1033:
guestfs-release-notes.txt:1035:
guestfs-release-notes.txt:1038:
guestfs-release-notes.txt:1041:
guestfs-release-notes.txt:1044:
guestfs-release-notes.txt:1048:
guestfs-release-notes.txt:1066:
guestfs-release-notes.txt:1069:
guestfs-release-notes.txt:1072:
guestfs-release-notes.txt:1074:
guestfs-release-notes.txt:1109:
guestfs-release-notes.txt:1112:
guestfs-release-notes.txt:1114:
guestfs-release-notes.txt:1116:
guestfs-release-notes.txt:1122:
guestfs-release-notes.txt:1126:
guestfs-release-notes.txt:1130:
guestfs-release-notes.txt:1135:
guestfs-release-notes.txt:1137:
guestfs-release-notes.txt:1140:
guestfs-release-notes.txt:1142:
guestfs-release-notes.txt:1145:
guestfs-release-notes.txt:1147:
guestfs-release-notes.txt:1150:
guestfs-release-notes.txt:1153:
guestfs-release-notes.txt:1156:
guestfs-release-notes.txt:1159:
guestfs-release-notes.txt:1161:
guestfs-release-notes.txt:1164:
guestfs-release-notes.txt:1167:
guestfs-release-notes.txt:1169:
guestfs-release-notes.txt:1175:
guestfs-release-notes.txt:1177:
guestfs-release-notes.txt:1180:
guestfs-release-notes.txt:1199:
guestfs-release-notes.txt:1202:
guestfs-release-notes.txt:1204:
guestfs-release-notes.txt:1207:
guestfs-release-notes.txt:1210:
guestfs-release-notes.txt:1212:
guestfs-release-notes.txt:1214:
guestfs-release-notes.txt:1216:
guestfs-release-notes.txt:1219:
guestfs-release-notes.txt:1222:
guestfs-release-notes.txt:1224:
guestfs-release-notes.txt:1227:
guestfs-release-notes.txt:1251:
guestfs-release-notes.txt:1255:
guestfs-release-notes.txt:1259:
guestfs-release-notes.txt:1261:
guestfs-release-notes.txt:1263:
guestfs-release-notes.txt:1266:
guestfs-release-notes.txt:1269:
guestfs-release-notes.txt:1272:
guestfs-release-notes.txt:1275:
guestfs-release-notes.txt:1278:
guestfs-release-notes.txt:1280:
guestfs-release-notes.txt:1286:
guestfs-release-notes.txt:1288:
guestfs-release-notes.txt:1291:
guestfs-release-notes.txt:1293:
guestfs-release-notes.txt:1296:
guestfs-release-notes.txt:1298:
guestfs-release-notes.txt:1300:
guestfs-release-notes.txt:1302:
guestfs-release-notes.txt:1304:
guestfs-release-notes.txt:1306:
guestfs-release-notes.txt:1308:
guestfs-release-notes.txt:1310:
guestfs-release-notes.txt:1312:
guestfs-release-notes.txt:1314:
guestfs-release-notes.txt:1316:
guestfs-release-notes.txt:1319:
guestfs-release-notes.txt:1322:
guestfs-release-notes.txt:1325:
guestfs-release-notes.txt:1328:
guestfs-release-notes.txt:1337:
guestfs-release-notes.txt:1415:
guestfs-release-notes.txt:1417:
guestfs-release-notes.txt:1420:
guestfs-release-notes.txt:1422:
guestfs-release-notes.txt:1424:
guestfs-release-notes.txt:1426:
guestfs-release-notes.txt:1428:
guestfs-release-notes.txt:1430:
guestfs-release-notes.txt:1435:
guestfs-release-notes.txt:1438:
guestfs-release-notes.txt:1442:
guestfs-release-notes.txt:1445:
guestfs-release-notes.txt:1448:
guestfs-release-notes.txt:1451:
guestfs-release-notes.txt:1455:
guestfs-release-notes.txt:1458:
guestfs-release-notes.txt:1460:
guestfs-release-notes.txt:1463:
guestfs-release-notes.txt:1465:
guestfs-release-notes.txt:1467:
guestfs-release-notes.txt:1469:
guestfs-release-notes.txt:1471:
guestfs-release-notes.txt:1473:
guestfs-release-notes.txt:1475:
guestfs-release-notes.txt:1477:
guestfs-release-notes.txt:1479:
guestfs-release-notes.txt:1481:
Binary file html/draft.png matches
Binary file logo/fish.png matches
Binary file tests/data/bin-i586-dynamic matches
Binary file tests/data/bin-sparc-dynamic matches
Binary file tests/data/bin-win32.exe matches
Binary file tests/data/bin-x86_64-dynamic matches
Binary file tests/data/filesanddirs-100M.tar.xz matches
Binary file tests/data/filesanddirs-10M.tar.xz matches
Binary file tests/data/helloworld.tar matches
Binary file tests/data/lib-i586.so matches
Binary file tests/data/lib-sparc.so matches
Binary file tests/data/lib-win32.dll matches
Binary file tests/data/lib-win64.dll matches
Binary file tests/data/lib-x86_64.so matches
Binary file tests/guests/guest-aux/minimal-hive matches
Binary file tests/guests/guest-aux/windows-software matches
Binary file tests/guests/guest-aux/windows-system matches
tools/virt-win-reg:711:
maint.mk: found trailing blank(s)
make: *** [sc_trailing_blank] Error 1
Binary files should probably not be matched.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
10:26 p.m.
On Sep 16, 2012, at 10:31 , Richard W.M. Jones wrote:
... and why they are (probably) not errors.
[snip]
> prohibit_magic_number_exit
> po-docs/ja.po:59861:"might be called indirectly from L<exit(3)>, which can
cause unexpected "
> po-docs/libguestfs-docs.pot:50902:"might be called indirectly from
L<exit(3)>, which can cause unexpected "
> po-docs/uk.po:57345:"might be called indirectly from L<exit(3)>, which can
cause unexpected "
> src/guestfs.pod:2028:callback might be called indirectly from L<exit(3)>, which
can cause
> tests/mount-local/test-parallel-mount-local.c:104: exit (77);
> tests/mount-local/test-parallel-mount-local.c:110: exit (77);
> tests/regressions/rhbz790721.c:79: exit (77);
> maint.mk: use EXIT_* values rather than magic number
> make: *** [sc_prohibit_magic_number_exit] Error 1
I think exit (77) is acceptable. To automake, this means that the
test has been skipped, and there is no constant for it. However
writing a regexp that matches all numbers except "77" is quite hard.
[snip]
Out of curiosity, why exit code 77 (permission denied) instead of exit code 75 (temporary
failure, user is invited to retry) [1]? Is this just an automake-ism?
--Alex
[1] For reference, here's a sysexits listing, consistent with what I saw in
/usr/include/sysexits.h on a Fedora 17 VM:
<http://www.opensource.apple.com/source/Libc/Libc-320/include/sysexits.h>
2:10 a.m.
On Sun, Sep 16, 2012 at 08:26:48PM -0700, Alex Nelson wrote:
On Sep 16, 2012, at 10:31 , Richard W.M. Jones wrote:
> ... and why they are (probably) not errors.
>
[snip]
>
>
>> prohibit_magic_number_exit
>> po-docs/ja.po:59861:"might be called indirectly from L<exit(3)>,
which can cause unexpected "
>> po-docs/libguestfs-docs.pot:50902:"might be called indirectly from
L<exit(3)>, which can cause unexpected "
>> po-docs/uk.po:57345:"might be called indirectly from L<exit(3)>,
which can cause unexpected "
>> src/guestfs.pod:2028:callback might be called indirectly from L<exit(3)>,
which can cause
>> tests/mount-local/test-parallel-mount-local.c:104: exit (77);
>> tests/mount-local/test-parallel-mount-local.c:110: exit (77);
>> tests/regressions/rhbz790721.c:79: exit (77);
>> maint.mk: use EXIT_* values rather than magic number
>> make: *** [sc_prohibit_magic_number_exit] Error 1
>
> I think exit (77) is acceptable. To automake, this means that the
> test has been skipped, and there is no constant for it. However
> writing a regexp that matches all numbers except "77" is quite hard.
[snip]
Out of curiosity, why exit code 77 (permission denied) instead of exit code 75 (temporary
failure, user is invited to retry) [1]? Is this just an automake-ism?
--Alex
[1] For reference, here's a sysexits listing, consistent with what I saw in
/usr/include/sysexits.h on a Fedora 17 VM:
<http://www.opensource.apple.com/source/Libc/Libc-320/include/sysexits.h>
That's a BSD thing, nothing to do with the automake code for
skipping a test.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
3:23 a.m.
Richard W.M. Jones wrote:
... and why they are (probably) not errors.
> bindtextdomain
> daemon/guestfsd.c
> erlang/erl-guestfs-proto.c
> examples/copy_over.c
> examples/create_disk.c
> examples/display_icon.c
> examples/inspect_vm.c
> examples/mount_local.c
> examples/virt-dhcp-address.c
> tests/c-api/test-add-drive-opts.c
> tests/c-api/test-add-libvirt-dom.c
> tests/c-api/test-command.c
> tests/c-api/test-config.c
> tests/c-api/test-create-handle.c
> tests/c-api/test-debug-to-file.c
> tests/c-api/test-just-header.c
> tests/c-api/test-last-errno.c
> tests/c-api/test-private-data.c
> tests/c-api/test-user-cancel.c
> tests/charsets/test-charset-fidelity.c
> tests/mount-local/test-parallel-mount-local.c
> tests/regressions/rhbz501893.c
> tests/regressions/rhbz790721.c
> maint.mk: the above files do not call bindtextdomain
> make: *** [sc_bindtextdomain] Error 1
You may want to disable that test, or to exempt all file names
under examples/ and tests/. To exempt those two directories,
you can add this to cfg.mk:
exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
I think for examples, tests and the rather special Erlang case,
it's
wrong to demand calls to bindtextdomain. The daemon is arguable, but
we don't include any locale data in the appliance.
> cast_of_argument_to_free
> generator/erlang.ml:403: pr " free ((char *)
optargs_s.%s);\n" n
> generator/ocaml.ml:588: pr " free ((char *) optargs_s.%s);\n"
n
> generator/python.ml:437: pr " free ((char **) optargs_s.%s);\n"
n
> maint.mk: don't cast free argument
> make: *** [sc_cast_of_argument_to_free] Error 1
It seems to me this is another shortcoming of 'const' in C.
The value of that syntax-check rule is decreasing rapidly,
with the trend in some projects to compile C code with a C++
compiler. There, the cast is required, and not anachronistic at all.
You might want to simply disable that test (by adding its name
to the list in cfg.mk's local-checks-to-skip).
Anyway, the cast seems necessary in the three cases above.
An alternative is to exempt those three files by adding this to cfg.mk:
exclude_file_name_regexp--sc_cast_of_argument_to_free = \
^generator/(erlang|ocaml|python).ml$$
> error_message_period
> php/README-PHP:33: echo ("Error: ".guestfs_last_error
($g)."\n");
> php/README-PHP:44: echo ("Error: ".guestfs_last_error
($g)."\n");
style guidelines for diagnostics in GNU programs
recommend against periods at end of diagnostics.
The above looks like a false positive.
The combination of regexps we use to match those uses heuristics
that (by definition) will not always work.
Exempt or disable. This is definitely a low-value syntax-check rule.
> php/extension/guestfs_php_002.phpt:30: die ("Error:
".guestfs_last_error ($g)."\n");
> php/extension/guestfs_php_002.phpt:35: echo ("Error:
".guestfs_last_error ($g)."\n");
> po-docs/ja.po-37794-"force>)."
> po-docs/libguestfs-docs.pot-33647-"I<--resize-force>)."
> po-docs/uk.po-35901-"force>)."
> resize/resize.ml:285: error (f_"%s: unknown partition table
type\nvirt-resize only supports MBR (DOS) and GPT partition tables.")
infile
> resize/resize.ml:685: error (f_"You cannot use --expand when there
is no surplus space to expand into. You need to make the target disk
larger by at least %s.")
> resize/resize.ml:697: error (f_"You cannot use --shrink when there
is no deficit (see 'deficit' in the virt-resize(1) man page).");
> resize/resize.ml:714: error (f_"There is a deficit of %Ld bytes
(%s). You need to make the target disk larger by at least this amount
or adjust your resizing requests.")
> src/launch-appliance.c:758: error (g, _("command failed: %s\nerrno:
%s\n\nIf qemu is located on a non-standard path, try setting the
LIBGUESTFS_QEMU\nenvironment variable. There may also be errors
printed above."),
> src/launch-libvirt.c-841- "'--format' option, or via the optional
format' argument to 'add-drive'."),
> src/launch.c-196- "This is a limitation of qemu."));
> src/launch.c-284- "This is a limitation of qemu."));
> src/libvirtdomain.c-571- "See ATTACHING TO RUNNING DAEMONS in
guestfs(3) for further information."));
> maint.mk: found error message ending in period
> make: *** [sc_error_message_period] Error 1
The PHP warning seems to be a bug in the script. The other cases are
acceptable because the error messages are proper sentences.
...
sysprep-operations.pod:@OPERATIONS@ \
> maint.mk: use $(...), not @...@
> make: *** [sc_makefile_at_at_check] Error 1
Real bug (in libguestfs). Podwrapper should be changed to use a
different character for its substitutions. There is a possibility
that these could be accidentally substituted by autoconf.
> prohibit_always-defined_macros
> cat/virt-ls.c:#define O_CLOEXEC 0
> daemon/daemon.h:#define O_CLOEXEC 0
> daemon/guestfsd.c:# define O_CLOEXEC 0
> examples/mount_local.c:#define O_CLOEXEC 0
> fish/fish.h:#define O_CLOEXEC 0
> generator/tests_c_api.ml:#define O_CLOEXEC 0
> src/guestfs-internal.h:#define O_CLOEXEC 0
> src/info.c:#define O_CLOEXEC 0
> tests/c-api/test-last-errno.c:#define O_CLOEXEC 0
> tests/c-api/test-user-cancel.c:#define O_CLOEXEC 0
> tests/xml/fake-libvirt-xml.c:#define O_CLOEXEC 0
> maint.mk: define the above via some gnulib .h file
> make: *** [sc_prohibit_always-defined_macros] Error 1
I don't think I understand what's wrong about this. Does gnulib
define O_CLOEXEC now?
Yes.
> prohibit_doubled_word
> po/pl.po:2947:do do
> maint.mk: doubled words
> make: *** [sc_prohibit_doubled_word] Error 1
Apparently 'do do' is OK in Polish ...
I'd exempt *.po files.
> prohibit_empty_lines_at_EOF
> contrib/intro/overview.png
> contrib/intro/tools.png
> contrib/intro/virt-manager-t.png
> contrib/intro/virt-manager.png
> contrib/intro/vmm-icons-t.png
> contrib/intro/vmm-icons.png
> contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch
> erlang/tests/010-load.erl
> guestfs-release-notes.txt
> html/draft.png
> logo/fish.png
> po/libguestfs.pot
> tests/data/bin-i586-dynamic
> tests/data/bin-sparc-dynamic
> tests/data/bin-win32.exe
> tests/data/bin-win64.exe
> tests/data/bin-x86_64-dynamic
> tests/data/filesanddirs-100M.tar.xz
> tests/data/filesanddirs-10M.tar.xz
> tests/data/helloworld.tar
> tests/data/helloworld.tar.gz
> tests/data/helloworld.tar.xz
> tests/data/known-4
> tests/data/known-5
> tests/data/lib-i586.so
> tests/data/lib-sparc.so
> tests/data/lib-win32.dll
> tests/data/lib-win64.dll
> tests/data/lib-x86_64.so
> tests/data/mbr-ext2-empty.img.gz
> tests/guests/guest-aux/debian-packages
> tests/guests/guest-aux/minimal-hive
> tests/guests/guest-aux/windows-software
> tests/guests/guest-aux/windows-system
> tests/regressions/rhbz576879.sh
> maint.mk: empty line(s) or no newline at EOF
> make: *** [sc_prohibit_empty_lines_at_EOF] Error 1
One problem with this test is it's matching binaries.
Right. It'd be good to have an automatic way to avoid considering
those, but I tend to VC so few binary files that it hasn't reached
that threshold.
> prohibit_magic_number_exit
> po-docs/ja.po:59861:"might be called indirectly from L<exit(3)>,
which can cause unexpected "
> po-docs/libguestfs-docs.pot:50902:"might be called indirectly from
L<exit(3)>, which can cause unexpected "
> po-docs/uk.po:57345:"might be called indirectly from L<exit(3)>,
which can cause unexpected "
> src/guestfs.pod:2028:callback might be called indirectly from
L<exit(3)>, which can cause
> tests/mount-local/test-parallel-mount-local.c:104: exit (77);
> tests/mount-local/test-parallel-mount-local.c:110: exit (77);
> tests/regressions/rhbz790721.c:79: exit (77);
> maint.mk: use EXIT_* values rather than magic number
> make: *** [sc_prohibit_magic_number_exit] Error 1
I think exit (77) is acceptable. To automake, this means that the
test has been skipped, and there is no constant for it. However
writing a regexp that matches all numbers except "77" is quite hard.
But we already have a secondary filtering mechanism, so the patch
to exempt "77" is pretty easy. If you confirm that this works for you,
I'll push it to gnulib:
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..1722eb7 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -354,7 +354,7 @@ sc_prohibit_strncpy:
# perl -pi -e 's/(^|[^.])\b(exit ?)\(0\)/$1$2(EXIT_SUCCESS)/'
sc_prohibit_magic_number_exit:
@prohibit='(^|[^.])\<(usage|exit|error) ?\(-?[0-9]+[,)]' \
- exclude='error ?\((0,|[^,]*)' \
+ exclude='exit \(77\)|error ?\(((0|77),|[^,]*)' \
halt='use EXIT_* values rather than magic number' \
$(_sc_search_regexp)
> prohibit_path_max_allocation
> daemon/initrd.c:41: char filename[PATH_MAX];
> daemon/initrd.c:126: char fullpath[PATH_MAX];
> daemon/inotify.c:310: char buf[PATH_MAX];
> daemon/link.c:38: char link[PATH_MAX];
> daemon/link.c:63: char link[PATH_MAX];
> daemon/realpath.c:86: char ret[PATH_MAX+1] = "/";
> daemon/xattr.c:272: char pathname[PATH_MAX];
> src/launch-appliance.c:179: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
> src/launch-libvirt.c:200: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
> src/launch-libvirt.c:224: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
> src/launch-unix.c:64: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
> maint.mk: Avoid stack allocations of size PATH_MAX
> make: *** [sc_prohibit_path_max_allocation] Error 1
The daemon ones are bugs in libguestfs. I haven't fixed them yet.
However the use of UNIX_PATH_MAX looks OK to me. I think the regexp
is over-matching.
Good catch.
The regexp in that test is too loose. This fixes it:
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..a6d1324 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1216,7 +1216,7 @@ sc_Wundef_boolean:
# not be constant, or might overflow a stack. In general, use PATH_MAX as
# a limit, not an array or alloca size.
sc_prohibit_path_max_allocation:
- @prohibit='(\balloca *\([^)]*|\[[^]]*)PATH_MAX' \
+ @prohibit='(\balloca *\([^)]*|\[[^]]*)\bPATH_MAX' \
halt='Avoid stack allocations of size PATH_MAX' \
$(_sc_search_regexp)
> prohibit_strcmp
This is getting long.
I'll continue in another message.
4:06 a.m.
On Mon, Sep 17, 2012 at 10:23:04AM +0200, Jim Meyering wrote:
>> prohibit_magic_number_exit
>> po-docs/ja.po:59861:"might be called indirectly from L<exit(3)>,
> which can cause unexpected "
>> po-docs/libguestfs-docs.pot:50902:"might be called indirectly from
> L<exit(3)>, which can cause unexpected "
>> po-docs/uk.po:57345:"might be called indirectly from L<exit(3)>,
> which can cause unexpected "
>> src/guestfs.pod:2028:callback might be called indirectly from
> L<exit(3)>, which can cause
>> tests/mount-local/test-parallel-mount-local.c:104: exit (77);
>> tests/mount-local/test-parallel-mount-local.c:110: exit (77);
>> tests/regressions/rhbz790721.c:79: exit (77);
>> maint.mk: use EXIT_* values rather than magic number
>> make: *** [sc_prohibit_magic_number_exit] Error 1
>
> I think exit (77) is acceptable. To automake, this means that the
> test has been skipped, and there is no constant for it. However
> writing a regexp that matches all numbers except "77" is quite hard.
But we already have a secondary filtering mechanism, so the patch
to exempt "77" is pretty easy. If you confirm that this works for you,
I'll push it to gnulib:
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..1722eb7 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -354,7 +354,7 @@ sc_prohibit_strncpy:
# perl -pi -e 's/(^|[^.])\b(exit ?)\(0\)/$1$2(EXIT_SUCCESS)/'
sc_prohibit_magic_number_exit:
@prohibit='(^|[^.])\<(usage|exit|error) ?\(-?[0-9]+[,)]' \
- exclude='error ?\((0,|[^,]*)' \
+ exclude='exit \(77\)|error ?\(((0|77),|[^,]*)' \
halt='use EXIT_* values rather than magic number' \
$(_sc_search_regexp)
Tested and confirmed that it works.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
4:08 a.m.
On Mon, Sep 17, 2012 at 10:23:04AM +0200, Jim Meyering wrote:
>> prohibit_path_max_allocation
>> daemon/initrd.c:41: char filename[PATH_MAX];
>> daemon/initrd.c:126: char fullpath[PATH_MAX];
>> daemon/inotify.c:310: char buf[PATH_MAX];
>> daemon/link.c:38: char link[PATH_MAX];
>> daemon/link.c:63: char link[PATH_MAX];
>> daemon/realpath.c:86: char ret[PATH_MAX+1] = "/";
>> daemon/xattr.c:272: char pathname[PATH_MAX];
>> src/launch-appliance.c:179: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
>> src/launch-libvirt.c:200: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
>> src/launch-libvirt.c:224: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
>> src/launch-unix.c:64: addr.sun_path[UNIX_PATH_MAX-1] = '\0';
>> maint.mk: Avoid stack allocations of size PATH_MAX
>> make: *** [sc_prohibit_path_max_allocation] Error 1
>
> The daemon ones are bugs in libguestfs. I haven't fixed them yet.
>
> However the use of UNIX_PATH_MAX looks OK to me. I think the regexp
> is over-matching.
Good catch.
The regexp in that test is too loose. This fixes it:
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..a6d1324 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1216,7 +1216,7 @@ sc_Wundef_boolean:
# not be constant, or might overflow a stack. In general, use PATH_MAX as
# a limit, not an array or alloca size.
sc_prohibit_path_max_allocation:
- @prohibit='(\balloca *\([^)]*|\[[^]]*)PATH_MAX' \
+ @prohibit='(\balloca *\([^)]*|\[[^]]*)\bPATH_MAX' \
halt='Avoid stack allocations of size PATH_MAX' \
$(_sc_search_regexp)
Tested and confirmed this patch works.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
3:47 a.m.
Richard W.M. Jones wrote:
...
cont'd
> prohibit_strcmp
> examples/mount_local.c:150: if (p && strcmp (p+1, "bash") ==
0) {
> examples/virt-dhcp-address.c:129: if (strcmp (guest_type, "linux") == 0)
{
> examples/virt-dhcp-address.c:134: if (strcmp (guest_distro, "fedora") ==
0 ||
> examples/virt-dhcp-address.c:135: strcmp (guest_distro, "rhel") == 0
||
> examples/virt-dhcp-address.c:136: strcmp (guest_distro,
"redhat-based") == 0) {
> examples/virt-dhcp-address.c:139: else if (strcmp (guest_distro,
"debian") == 0 ||
> examples/virt-dhcp-address.c:140: strcmp (guest_distro, "ubuntu") ==
0) {
> examples/virt-dhcp-address.c:151: else if (strcmp (guest_type,
"windows") == 0) {
> test-tool/test-tool.c:43://#define STRNEQ(a,b) (strcmp((a),(b)) != 0)
> maint.mk: maint.mk: replace strcmp calls above with STREQ/STRNEQ
> make: *** [sc_prohibit_strcmp] Error 1
We want to use strcmp in examples. They don't follow the ordinary
code conventions used in the rest of libguestfs.
I would exempt ^examples/ from this check.
There's something wrong with check matching test-tool.c.
Thank you. That is another regexp that can be improved:
[the ":" was trying to anchor that "#" to beginning of line
in the grep-style "FILE:...text" output that it's filtering,
but that is wrong for two reasons: your example is one, but the
other is that the "#" need not be at the beginning of the line.]
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..f77d0c1 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -330,7 +330,7 @@ sc_prohibit_atoi_atof:
sp_ = strcmp *\(.+\)
sc_prohibit_strcmp:
@prohibit='! *strcmp *\(|\<$(sp_) *[!=]=|[!=]= *$(sp_)' \
- exclude=':# *define STRN?EQ\(' \
+ exclude='# *define STRN?EQ\(' \
halt='replace strcmp calls above with STREQ/STRNEQ' \
$(_sc_search_regexp)
> prohibit_strcmp_and_strncmp
> examples/mount_local.c:150: if (p && strcmp (p+1, "bash") ==
0) {
...
> maint.mk: use STREQ(LEN) in place of the above uses of
strcmp(strncmp)
> make: *** [sc_prohibit_strcmp_and_strncmp] Error 1
Examples should be excluded.
You got that right ;-)
> prohibit_strncpy
> src/launch-appliance.c:178: strncpy (addr.sun_path, guestfsd_sock,
UNIX_PATH_MAX);
> src/launch-libvirt.c:199: strncpy (addr.sun_path, guestfsd_sock,
UNIX_PATH_MAX);
> src/launch-libvirt.c:223: strncpy (addr.sun_path, console_sock, UNIX_PATH_MAX);
> src/launch-unix.c:63: strncpy (addr.sun_path, sockpath, UNIX_PATH_MAX);
> maint.mk: do not use strncpy, period
> make: *** [sc_prohibit_strncpy] Error 1
I think use of strncpy is justified here.
I agree. I would exempt those two files.
> prohibit_test_minus_ao
> appliance/init:26:if [ ! -L /etc/init.d/udev -a -x /etc/init.d/udev ]; then
> configure.ac:1276: [test "x$GOBJECT_LIBS" != "x" -a
"x$GIO_LIBS" !=
"x"])
> edit/test-virt-edit.sh:32:if [ "$(../cat/virt-cat -a test.qcow2
/etc/test3)" != "a
> edit/test-virt-edit.sh:47: if [ "$(../cat/virt-cat -a test.qcow2
/etc/test3)" != "1
> edit/test-virt-edit.sh:61:if [ "$(../fish/guestfish -i -a test.qcow2
--ro lstat /etc/test3 | grep -E '^(mode|uid|gid):' | sort)" != "gid:
11
> fish/test-mount-local.sh:36:if [ $# -gt 0 -a "$1" = "--run-test"
]; then
> format/test-virt-format.sh:29:if [ "$(../cat/virt-filesystems -a
test1.img)" != "/dev/sda1" ]; then
> fuse/test-fuse-umount-race.sh:75:if [ "$(../fish/guestfish -a
test-copy.qcow2 --ro -i is-file /test-umount)" != "true" ]; then
> fuse/test-fuse.sh:55:if [ ! -x "$guestfish" -o ! -x "$guestmount"
]; then
> src/api-support/update-from-tarballs.sh:39: if [ $v != "1.2.0" -a $v
!= "1.3.0" -a ! -f $v ]; then
> sysprep/test-virt-sysprep-script.sh:40:if [ ! -f stamp-script1.sh -o
! -f stamp-script2.sh ]; then
> tests/md/test-inspect-fstab-md.sh:33:if [ -z "$f" -o ! -f "$f" ];
then
> tools/test-virt-make-fs.sh:48:if [ "$ntfs3g_available" = "yes"
-a
"$ntfsprogs_available" = "yes" ]; then
> maint.mk: use "test C1 && test C2", not "test C1 -a C2";
use "test
C1 || test C2", not "test C1 -o C2"
> make: *** [sc_prohibit_test_minus_ao] Error 1
What was wrong with -a and -o? Perhaps we should exempt this test for
scripts that explicitly begin #!/bin/bash, since presumably these will
use bash's built-in test.
This is well documented in autoconf's "Limitations of Builtins"
documentation:
`test'
The `test' program is the way to perform many file and string
tests. It is often invoked by the alternate name `[', but using
that name in Autoconf code is asking for trouble since it is an M4
quote character.
The `-a', `-o', `(', and `)' operands are not present in all
implementations, and have been marked obsolete by Posix 2008.
This is because there are inherent ambiguities in using them. For
example, `test "$1" -a "$2"' looks like a binary operator to
check
whether two strings are both non-empty, but if `$1' is the literal
`!', then some implementations of `test' treat it as a negation of
the unary operator `-a'.
Thus, portable uses of `test' should never have more than four
arguments, and scripts should use shell constructs like `&&' and
`||' instead. If you combine `&&' and `||' in the same
statement,
keep in mind that they have equal precedence, so it is often
better to parenthesize even when this is redundant. For example:
# Not portable:
test "X$a" = "X$b" -a \
'(' "X$c" != "X$d" -o "X$e" =
"X$f" ')'
# Portable:
test "X$a" = "X$b" &&
{ test "X$c" != "X$d" || test "X$e" =
"X$f"; }
`test' does not process options like most other commands do; for
example, it does not recognize the `--' argument as marking the
end of options.
It is safe to use `!' as a `test' operator. For example, `if test
! -d foo; ...' is portable even though `if ! test -d foo; ...' is
not.
> prohibit_trailing_blank_lines
> contrib/visualize-alignment/qemu-0.13-trace-block-device-access.patch
> guestfs-release-notes.txt
> po/libguestfs.pot
> tests/guests/guest-aux/debian-packages
> maint.mk: found trailing blank line(s)
> make: *** [sc_prohibit_trailing_blank_lines] Error 1
guestfs-release-notes.txt and po/libguestfs.pot are generated files.
I would exempt those files or
change the generation rules to filter out trailing blanks.
Of course, it's even better not to VC generated files,
but I suppose you have no choice because the tools to
perform the generation are not guaranteed to be available.
debian-packages is effectively a binary file.
exempt it, I suppose,
though it doesn't look like a binary to me.
> prohibit_undesirable_word_seq
> BUGS:147:can not
...
> maint.mk: undesirable word sequence
> make: *** [sc_prohibit_undesirable_word_seq] Error 1
Pet peeve ;-)
> require_config_h
> examples/copy_over.c
> examples/create_disk.c
> examples/display_icon.c
> examples/inspect_vm.c
> examples/mount_local.c
> examples/virt-dhcp-address.c
> tests/c-api/test-just-header.c
> maint.mk: the above files do not include <config.h>
> make: *** [sc_require_config_h] Error 1
Examples shouldn't include <config.h>. The test-just-header.c test
needs to not include it, because of what it is testing.
Yes, another case of exemption-required.
Yeah, this can be a pain (especially if you do it all at once), but the
consolation is that you have to do it only once. Any subsequent
failures typically indicate real problems or are quickly dispatched
via another exemption.
> require_config_h_first
> examples/copy_over.c
> examples/create_disk.c
> examples/display_icon.c
> examples/inspect_vm.c
> examples/mount_local.c
> examples/virt-dhcp-address.c
> tests/c-api/test-just-header.c
> maint.mk: the above files include some other header before <config.h>
> make: *** [sc_require_config_h_first] Error 1
> trailing_blank
> TODO:405: - swap devices (both of block device and file) should be
wiped. This may
> Binary file tests/guests/guest-aux/windows-software matches
> Binary file tests/guests/guest-aux/windows-system matches
> tools/virt-win-reg:711:
> maint.mk: found trailing blank(s)
> make: *** [sc_trailing_blank] Error 1
Binary files should probably not be matched.
Hey!
I've just realized that I can easily filter out those lines.
Here's yet another patch.
We should be able to use a similar change for a few other rules.
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..ccf09a2 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -724,6 +724,7 @@ sc_require_test_exit_idiom:
sc_trailing_blank:
@prohibit='[ ]$$' \
halt='found trailing blank(s)' \
+ exclude='^Binary file .* matches$$' \
$(_sc_search_regexp)
# Match lines like the following, but where there is only one space
Thanks for the detailed report!
4:10 a.m.
On Mon, Sep 17, 2012 at 10:47:52AM +0200, Jim Meyering wrote:
> There's something wrong with check matching test-tool.c.
Thank you. That is another regexp that can be improved:
[the ":" was trying to anchor that "#" to beginning of line
in the grep-style "FILE:...text" output that it's filtering,
but that is wrong for two reasons: your example is one, but the
other is that the "#" need not be at the beginning of the line.]
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..f77d0c1 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -330,7 +330,7 @@ sc_prohibit_atoi_atof:
sp_ = strcmp *\(.+\)
sc_prohibit_strcmp:
@prohibit='! *strcmp *\(|\<$(sp_) *[!=]=|[!=]= *$(sp_)' \
- exclude=':# *define STRN?EQ\(' \
+ exclude='# *define STRN?EQ\(' \
halt='replace strcmp calls above with STREQ/STRNEQ' \
$(_sc_search_regexp)
Test and confirmed working.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
4:17 a.m.
On Mon, Sep 17, 2012 at 10:47:52AM +0200, Jim Meyering wrote:
>> trailing_blank
>> TODO:405: - swap devices (both of block device and file) should be
> wiped. This may
>> Binary file tests/guests/guest-aux/windows-software matches
>> Binary file tests/guests/guest-aux/windows-system matches
>> tools/virt-win-reg:711:
>> maint.mk: found trailing blank(s)
>> make: *** [sc_trailing_blank] Error 1
>
> Binary files should probably not be matched.
Hey!
I've just realized that I can easily filter out those lines.
Here's yet another patch.
We should be able to use a similar change for a few other rules.
diff --git a/top/maint.mk b/top/maint.mk
index 4627bc5..ccf09a2 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -724,6 +724,7 @@ sc_require_test_exit_idiom:
sc_trailing_blank:
@prohibit='[ ]$$' \
halt='found trailing blank(s)' \
+ exclude='^Binary file .* matches$$' \
$(_sc_search_regexp)
# Match lines like the following, but where there is only one space
Yes, this works to exclude the false binary matches.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
2:05 a.m.
Richard W.M. Jones wrote:
On Mon, Sep 17, 2012 at 10:47:52AM +0200, Jim Meyering wrote:
> >> trailing_blank
> >> TODO:405: - swap devices (both of block device and file) should be
> > wiped. This may
> >> Binary file tests/guests/guest-aux/windows-software matches
> >> Binary file tests/guests/guest-aux/windows-system matches
> >> tools/virt-win-reg:711:
> >> maint.mk: found trailing blank(s)
> >> make: *** [sc_trailing_blank] Error 1
> >
> > Binary files should probably not be matched.
>
> Hey!
> I've just realized that I can easily filter out those lines.
> Here's yet another patch.
> We should be able to use a similar change for a few other rules.
>
> diff --git a/top/maint.mk b/top/maint.mk
> index 4627bc5..ccf09a2 100644
> --- a/top/maint.mk
> +++ b/top/maint.mk
> @@ -724,6 +724,7 @@ sc_require_test_exit_idiom:
> sc_trailing_blank:
> @prohibit='[ ]$$' \
> halt='found trailing blank(s)' \
> + exclude='^Binary file .* matches$$' \
> $(_sc_search_regexp)
>
> # Match lines like the following, but where there is only one space
Yes, this works to exclude the false binary matches.
Thanks for testing those.
I've pushed the changes to gnulib:
Subject: 0/4: syntax-check: tweak for fewer false positives
Newsgroups: gmane.comp.lib.gnulib.bugs
Richard W.M. Jones went through the (tedious!) process of reviewing
all of libguestfs wrt gnulib's latest syntax-check rules, and reported
many false positives. In a few cases, I found ways to avoid entire
classes of false positives:
1/4 maint.mk: relax sc_prohibit_strcmp, to avoid a false positive
2/4 maint.mk: teach sc_prohibit_magic_number_exit to accept 77
3/4 maint.mk: sc_prohibit_path_max_allocation: don't FP for UNIX_PATH_MAX
4/4 maint.mk: exempt trailing blanks found in "binary" files
4447
days inactive
4449
days old
9 comments
3 participants
participants (3)
-
Alex Nelson -
Jim Meyering -
Richard W.M. Jones