Run this command across the source:
perl -pi.bak -e 's/(20[012][0-9])-20[12][012]/$1-2023/g' `git ls-files`
and remove changes to po{,-docs}/*.po{,t} (these will be regenerated
later when we run 'make dist').
Factor out the common code used to hexdump the protocol when
./configure --enable-packet-dump is used.
It's not quite a straight refactor because I had to fix some
signedness bugs in the code which were not revealed before because
this code is usually disabled.
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.)
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.
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.
Updating gnulib has caused -Wformat-signedness to be enabled. This
has revealed many problems in C format strings. The fixes here fall
into the following main categories:
- Using %d with an unsigned parameter.
- %x and %o expect an unsigned argument.
- uid_t and gid_t are unsigned on Linux. The safe way to print these
is to cast them to uintmax_t and then print them using the %ju
modifier (see http://stackoverflow.com/a/1401581).
- Using %d to print an enum. Since enums may be either char or int,
I fixed this by casting the enum to int.
- strtol_error & lzma_ret are both unsigned types.
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
There are now two forms of the 'debug progress' command:
(1) debug progress <n> (the original form) generates ordinary
rate-limited progress messages for <n> seconds.
(2) debug progress <n> <rate> generates progress messages every <rate>
microseconds for <n> seconds.
The second form omit the usual rate-limiting, and so wouldn't
be generated like this from an ordinary API call. However this
is useful for testing events (see RHBZ#909624).
This function allows you to pass an explicit errno back to the
library. reply_with_error is redefined as a macro that calls
reply_with_error_errno with errno == 0.
There is another case where downloads of small files could fail if the
library side (writer) fails. In this case the library would send back
a cancellation, but it would be received after the daemon had finished
sending the whole file (because the file is small enough). The daemon
would reenter the main loop and immediately get an unexpected cancel
message, causing the daemon to die.
This commit also makes test-cancellation-download-librarycancels.sh
more robust. We use Monte-Carlo testing with a range of file sizes.
Small file sizes should trigger the error case.
Remove some debug messages which were basically left over from when
the code was being developed.
However we leave debug messages where it is printing an external
command that is about to be executed, since those are useful.
This introduces a new form of progress event, where we don't know how
much of the operation has taken place, but we nevertheless want to
send back some indication of activity. Some progress bar indicators
directly support this, eg. GtkProgressBar where it is known as "pulse
mode".
A pulse mode progress message is a special backwards-compatible form
of the ordinary progress message. No change is required in callers,
unless they want to add support for pulse mode.
The daemon sends:
- zero or more progress messages with position = 0, total = 1
- a single final progress message with position = total = 1
Note that the final progress message may not be sent if the call fails
and returns an error. This is consistent with the behaviour of
ordinary progress messages.
The daemon allows two types of implementation. Either you can just
call notify_progress (0, 1); ...; notify_progress (1, 1) as usual.
Or you can call the functions pulse_mode_start, pulse_mode_end and/or
pulse_mode_cancel (see documentation in daemon/daemon.h). For this
second form of call, the guarantee is very weak: it *just* says the
daemon is still capable of doing something, and it doesn't imply that
if there is a subprocess that it is doing anything. However this does
make it very easy to add pulse mode progress messages to all sorts of
existing calls that depend on long-running external commands.
To do: add a third variant that monitors a subprocess and only sends
back progress messages if it's doing something, where "doing
something" might indicate it's using CPU time or it's printing output.
This adds 'guestfsd: ...' prefix before each message, and
also puts a message at the top of the main loop just after
a new message has been received.
The intent is to make it simpler to follow the protocol.
The chunk.cancel field should always be [0|1]. If it is not then
something has gone badly wrong -- probably loss of synchronization.
If this occurs print a debug message and return error from
receive_file function.
Previously we only supported optional arguments for library
functions (commit 14490c3e1a).
This extends that work so that optional arguments can also be
passed through to the daemon.
If the daemon sends progress notification messages while we
are uploading FileIn parameters, these are received in
check_for_daemon_cancellation_or_eof. Modify this library
function so that it turns these messages into callbacks.
Two unrelated changes to the protocol to support progress
messages during uploads, and optional arguments.
Note that this makes an incompatible change to the protocol,
and this is reflected in the protocol version field (3 -> 4).
This changes the protocol again so that if the errno is available,
it is converted to a string (like "EIO") and sent back over the
protocol to the library.
In this commit the library just discards the string.
