Adding an exception for the whole file usually defeats the purpose of a
syntax check and is also likely to get forgotten once the file is
removed.
In case of the suggestion of using 'safewrite' instead of write even the
comment for safewrite states that the function needs to be used only in
certain cases.
Remove the blanket exceptions for files and use an exclude string
instead. The only instance where we keep the full file exception is for
src/libvirt-stream.c as there are multiple uses in example code in
comments where I couldn't find a nicer targetted wapproach.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This prevents starting any daemons with improper logging settings. This is
desirable on its own, but will be even more beneficial when more functions start
reporting errors and failing on them, coming up in following patches
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Move the unlinking of the state file earlier and get rid of the cleanup
label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Convert two temp strings and one virJSONValue to g_auto(free|ptr).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virJSONValueObjectAdd instead of step-by-step construction of the
object. This also removes a handful impossible to reach errors with
translatable messages.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead.
In few instances it allows us to also remove explicit clearing of
pointers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In a few places we declare a variable (which is optionally
followed by a code not touching it) then set the variable to a
value and return the variable immediately. It's obvious that the
variable is needless and the value can be returned directly
instead.
This patch was generated using this semantic patch:
@@
type T;
identifier ret;
expression E;
@@
- T ret;
... when != ret
when strict
- ret = E;
- return ret;
+ return E;
After that I fixed couple of formatting issues because coccinelle
formatted some lines differently than our coding style.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit 94e45d1042e broke exec-restart of virtlogd and virtlockd as the
code waiting for the daemon shutdown closed the daemons before
exec-restarting.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912243
Fixes: 94e45d1042e
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Generated using the following spatch:
@@
expression path;
@@
- virFileMakePath(path)
+ g_mkdir_with_parents(path, 0777)
However, 14 occurrences were not replaced, e.g. in
virHostdevManagerNew(). I don't really understand why.
Fixed by hand afterwards.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The parent array takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Initialize and free @magic since virJSONValueObjectAppendString
does not free it for us eventually.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Don't hide our use of GHashTable behind our typedef. This will also
promote the use of glibs hash function directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
All but one of the callers either use the list in arbitrary order or
sorted by key. Rewrite the function so that it supports sorting by key
natively and make it return the element count. This in turn allows to
rewrite the only caller to sort by value internally.
This allows to remove multiple sorting functions which were sorting by
key and the function will be also later reused for some hash operations
internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
All users of virHashTable pass strings as the name/key of the entry.
Make this an official requirement by turning the variables to 'const
char *'.
For any other case it's better to use glib's GHashTable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
It doesn't make much sense to configure the bucket count in the hash
table for each case specifically. Replace all calls of virHashCreate
with virHashNew which has a pre-set size and remove virHashCreate
completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
There are few places where a return variable is introduced (ret
or retval), but then is never changed and is then passed to
return. Well, we can return the value that the variable is
initialized to directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When using systemd we want to take advantage of socket activation
instead of keeping daemons running all the time, so we default to
shutting them down after two minutes of inactivity.
At the same time, we want it to be possible for the admin to opt
out of this behavior and disable timeouts entirely. A very natural
way to do so would be to specify a zero-length timeout, but that's
currently not accepted by the command line parser. Address that.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make it obvious that the function always returns a valid pointer and fix
all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This addreses portability to Windows and standardizes
error reporting. This fixes a number of places which
failed to set O_CLOEXEC or failed to report errors.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Use the glib allocation function that never returns NULL and remove the
now dead-code checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:
commit 49288fac965f0ee23db45d83ae4ef3a9a71dafd0
Author: Peter Krempa <pkrempa@redhat.com>
Date: Wed Oct 9 15:26:37 2019 +0200
util: hash: Add possibility to use simpler data free function in virHash
It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.
After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.
We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Replace all occurrences of
if (VIR_STRDUP(a, b) < 0)
/* effectively dead code */
with:
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f029159ed701b4a1739ac711507ee53790ed
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOPTR aliases to g_autoptr. Replace all of its use by the GLib
macro version.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The usleep function was missing on older mingw versions, but we can rely
on it existing everywhere these days. It may only support times upto 1
second in duration though, so we'll prefer to use g_usleep instead.
The commandhelper program is not changed since that can't link to glib.
Fortunately it doesn't need to build on Windows platforms either.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Neither virThreadInitialize or virThreadOnExit do anything since we
dropped the Win32 threads impl, in favour of win-pthreads with:
commit 0240d94c36c8ce0e7c35b5be430acd9ebf5adcfa
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Wed Jan 22 16:17:10 2014 +0000
Remove windows thread implementation in favour of pthreads
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The Perl bindings for libvirt use the test driver for unit tests. This
tries to load the cpu_map/index.xml file, and when run from an
uninstalled build will fail.
The problem is that virFileActivateDirOverride is called by our various
binaries like libvirtd, virsh, but is not called when a 3rd party app
uses libvirt.so
To deal with this we allow the LIBVIRT_DIR_OVERRIDE=1 env variable to be
set and make virInitialize look for this. The 'run' script will set it,
so now build using this script to run against an uninstalled tree we
will correctly resolve files to the source tree.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
All code using LOCALSTATEDIR "/run" is updated to use RUNSTATEDIR
instead. The exception is the remote driver client which still
uses LOCALSTATEDIR "/run". The client needs to connect to remote
machines which may not be using /run, so /var/run is more portable
due to the /var/run -> /run symlink.
Some duplicate paths in the apparmor code are also purged.
There's no functional change by default yet since both expressions
expand to the same value.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Instead of having each caller pass in the desired logfile name, pass in
the binary name instead. The logging code can then just derive a logfile
name by appending ".log".
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Using the new system activation APIs allows for simpler code setting up
the network services.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently the socket code will unlink any UNIX socket path which is
associated with a server socket. This is not fine grained enough, as we
need to avoid unlinking server sockets we were passed by systemd.
To deal with this we must explicitly track whether each socket needs to
be unlinked when closed, separately of the client vs server state.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The virNetServerServiceNewFD API only accepts a single FD, but it is
easily changed to allow for an array of FDs to be passed in.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
