Just code motion.
This commit makes it clearer what is a utility and what is part of the
library. It also makes it clear that we should rename:
guestfs-internal-frontend.h -> utils.h
guestfs-internal-frontend-cleanups.h -> cleanups.h (?)
but this commit does not make that change.
Run the following command over the source:
perl -pi.bak -e 's/(20[01][0-9])-2016/$1-2017/g' `git ls-files`
(Thanks Rich for the perl snippet, as used in past years.)
When TESTS_ENVIRONMENT already uses 'run', the VG variable
doesn't also need to use 'run'.
The specific problem is that if the command contains newlines
then double invocations of the 'run' script fails (in libtool).
ie the following command failed causing errors in check-valgrind:
$VG virt-builder phony-fedora \
-v --no-cache --no-check-signature $no_network \
...
--write '/etc/append4:line1
' \
...
Create a new libfishcommon convenience static library to build just once
(instead of 12 times!) the majority of the guestfish sources used in the
rest of the C tools (mostly for command line stuff, inspection, and
mount).
The notable exceptions not using libfishcommon are guestfish itself, and
virt-rescue: both need to build at least one of the common sources using
additional CPPFLAGS.
Some of the C tools were building also config.c as part of the shared
sources from guestfish, and thus bringing a dependency on libconfig.
Since none of them actually read the libguestfs configuration at all,
then exclude fish/config.c from their build, and stop linking to
libconfig.
Make use of the recently added 'getprogname' module in gnulib: replace
our guestfs_int_program_name with the getprogname() provided by the
module, since it does the same thing, and in a portable way.
As consequence of the above, use gnulib in a couple of tests that use
getprogname().
Since guestfs_int_program_name is gone, drop the configure checks
associated with it.
By adding common CLEANFILES and DISTCLEANFILES variables to
common-rules.mk, we can remove these from most other Makefiles, and
also clean files more consistently.
Note that bin_PROGRAMS are already cleaned by 'make clean', so I
removed cases where these were unnecessarily added to CLEANFILES.
By moving these two functions out of the common options parsing code,
it means we don't need to depend on all the other machinery of options
parsing, such as the global variables ("verbose"), libconfig, etc.
For OCaml tools this does essentially nothing useful because the
--help output is automatically generated from the options, and so
cannot be wrong. However for C tools this is a useful check.
It would be nice to generate C tools --help output, but there isn't
enough information in the getopt data to do that.
This commit also includes fixes to the --help output for a few tools.
podcheck.pl is run as part of the tests to perform various checks on
the documentation and the tool.
Currently we check only that the documented options matches the
options that the tool implements and vice versa. This commit would
also allow us (in future) to check --help, --long-options,
--short-options, --version output.
This commit includes scripts to run the tests and various fixes to the
manual pages to ensure that the tests pass.
When possible, make the disk image format explicit when invoking tools
or using add-drive. This avoids warnings from qemu about the unspecified
format for the image, and also makes qemu slightly faster (skipping the
disk image probing).
Tests checking the image probing are not touched.
This changes also:
- old-style invocations of tools (`$tool $filename`) into new style
(`$tool -a $filename`)
- add-drive-ro/add-drive-with-if guestfish commands into add/add-drive
with explicit readonly/iface arguments
There should be no change in the tests results.
Like with the previous commit, this replaces instances of:
if (something_bad) {
fprintf (stderr, "%s: error message\n", guestfs_int_program_name);
exit (EXIT_FAILURE);
}
with:
if (something_bad)
error (EXIT_FAILURE, 0, "error message");
(except in a few cases were errno was incorrectly being ignored, in
which case I have fixed that).
It's slightly more complex than the previous commit because we must be
careful to:
- Remove the program name (since error(3) prints it).
- Remove any trailing \n character from the message.
Candidates for replacement were found using:
pcregrep --buffer-size 10M -M '\bfprintf\b.*\n.*\bexit\b' `git ls-files`
Wherever we had code which did:
if (something_bad) {
perror (...);
exit (EXIT_FAILURE);
}
replace this with use of the error(3) function:
if (something_bad)
error (EXIT_FAILURE, errno, ...);
The error(3) function is supplied by glibc, or by gnulib on platforms
which don't have it, and is much more flexible than perror(3). Since
we already use error(3), there seems to be no downside to mandating it
everywhere.
Note there is one nasty catch with error(3): error (EXIT_SUCCESS, ...)
does *not* exit! This is also the reason why error(3) cannot be
marked as __attribute__((noreturn)).
Because the examples can't use gnulib, I did not change them.
To search for multiline patterns of the above form, pcregrep -M turns
out to be very useful:
pcregrep --buffer-size 10M -M '\bperror\b.*\n.*\bexit\b' `git ls-files`
GCC has two warnings related to large stack frames. We were already
using the -Wframe-larger-than warning, but this reduces the threshold
from 10000 to 5000 bytes.
However that warning only covers the static part of frames (not
alloca). So this change also enables -Wstack-usage=10000 which covers
both the static and dynamic usage (alloca and variable length arrays).
Multiple changes are made throughout the code to reduce frames to fit
within these new limits.
Note that stack allocation of large strings can be a security issue.
For example, we had code like:
size_t len = strlen (fs->windows_systemroot) + 64;
char software[len];
snprintf (software, len, "%s/system32/config/software",
fs->windows_systemroot);
where fs->windows_systemroot is guest controlled. It's not clear what
the effects might be of allowing the guest to allocate potentially
very large stack frames, but at best it allows the guest to cause
libguestfs to segfault. It turns out we are very lucky that
fs->windows_systemroot cannot be set arbitrarily large (see checks in
is_systemroot).
This commit changes those to large heap allocations instead.
Remove man pages and other pages which 'make clean' did not remove
before.
To evaluate which pages could be removed, I did a full build and
check, and then ran 'make clean' followed by 'git clean -xdf'. By
examining the output of the git clean command I could see which files
were being missed.
Files that are _not_ removed by make clean or make distclean:
- generator-built files
- Makefile, Makefile.in, .deps, .depend
- any ./configure output files (maybe they should be?)
Move the random set of HTML files we build from html/ into
the website/ directory.
Also in the website/ directory, put the index.html file from
http://libguestfs.org, which was previously not under version control.
It is generated from index.html.in so we can automatically add the
current version and release date.
Also in the website/ directory, put various CSS file, images, etc.
which are required by the website and were also previously not under
version control.
Change the 'make website' rule to 'make maintainer-upload-website'.
As the name suggests, it is only useful for the maintainer, and will
fail with an error for anyone else.
Create a new top-level directory called test-data, which will carry
all the test data which is large and/or shared between multiple tests.
There are actually several new subdirectories created:
test-data/binaries: The pre-built binary and library files for random
architectures that we use to test various architecture detection
features (was part of tests/data).
test-data/blank-disks: The blank disks which are used for disk format
detection (was part of tests/data).
test-data/files: Other miscellaneous test files from tests/data that
are not included in the above.
test-data/phony-guests: The phony guests (was tests/guests).
test-data: The top-level directory builds the 'test.iso' image file
that is used for testing the C API and in miscellaneous other tests.
Because of previous automated commits, such as changing 'guestfs___'
-> 'guestfs_int_', several function calls no longer lined up with
their parameters, and some lines were too long.
The bulk of this commit was done using emacs batch mode and the
technique described here:
http://www.cslab.pepperdine.edu/warford/BatchIndentationEmacs.html
The changes suggested by emacs were then reviewed by hand.
Done using a sequence of regular expressions like this:
perl -pi.bak -e 's{C</}{F</}g' `git ls-files \*.pod` generator/actions.ml
perl -pi.bak -e 's{C<C:\\}{F<C:\\}g' `git ls-files \*.pod` generator/actions.ml
[etc]
and then tediously checking every change by hand.
The man page for getline says:
ssize_t getline(char **lineptr, size_t *n, FILE *stream);
[...]
If *lineptr is set to NULL and *n is set 0 before the call, then get‐
line() will allocate a buffer for storing the line. This buffer should
be freed by the user program even if getline() failed.
which seems to indicate that we must initialize both line and len to 0
before the first call to getline.
In several places we were not initializing len. The program still
worked fine, but it seems better to initialize the length anyway.
libguestfs has used double and triple underscores in identifiers.
These aren't valid for global names in C++.
The first step is to replace all guestfs___* (3 underscores) with
guestfs_int_*. We've used guestfs_int_* elsewhere already as a prefix
for internal identifiers.
This is an entirely mechanical change done using:
git ls-files | xargs perl -pi.bak -e 's/guestfs___/guestfs_int_/g'
Reference: http://stackoverflow.com/a/228797
The gnulib 'error' module uses 'program_name'. On some platforms --
but not Linux / glibc -- it references it as:
extern char *program_name;
This means when you compile libguestfs on non-glibc (eg. Mac OS X)
gnulib requires 'program_name' as an external string reference, which
we don't provide.
This change doesn't define this string reference for gnulib, but it
does change the name of the macro we use to avoid conflicts if we
eventually need to export 'program_name' as a string.
Thanks: Margaret Lewicka
This API already existed (as guestfs___add_libvirt_dom), and was used
by a few tools.
This commit changes it to a public API.
Note that for reasons outlined in the previous commit message, it is
impossible to call this from guestfish or from non-C language
bindings.
Just like --long-options, it makes it possible to know which short
options are supported by each tool; this can help improving the bash
completion, for example.
Since procps 3.3.10, free does not output the "-/+ buffers/cache" line
anymore. On the other hand, the data from it can be calculated from the
memory values, so just do the calculation manually (with awk).
All tests run under the ./run binary. For a long time the ./run
binary has set the $PATH environment variable to contain all of the
directories with binaries in them.
Therefore there is no reason to use ../fish/guestfish instead of just
plain guestfish (and the same applies to other built binaries).
In most C tools, virt-sysprep and virt-customize, you have to put the
--format parameter before the corresponding -a parameter. ie. The
following is correct:
guestfish --format qcow2 -a disk1 -a disk2
But the following is incorrect. The --format parameter is dangling
and prior to this commit would have been silently ignored:
guestfish -a disk1 -a disk2 --format qcow2
After this change, dangling --format parameters now lead to an error:
guestfish: --format parameter must appear before -a parameter
In virt-customize, also check that --attach-format parameter appears
before --attach parameter.
Thanks: Lingfei Kong
If pthread_cond_wait fails and the thread worker ends, make sure to
unlock retire_mutex, which has been locked for the condition.
The return value of pthread_mutex_unlock is ignored, as there's nothing
that can be done with it at that point.
This changes podwrapper so that the input (POD) files should not
contain an =encoding directive. However they must be UTF-8.
Podwrapper then adds the '=encoding utf8' directive back during final
generation.
This in particular avoids problems with nested =encoding directives in
fragments. These break POD, and are undesirable anyway.
