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. _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs