Thu, 3 Aug 2023 21:38:02 +0200 Michal Prívozník :
> it looks like we're using { 0 } everywhere else so that's why I went with it.
I was under the impression {} is valid. But apparently it is not,
so I learned something new today: "ISO C forbids empty initializer braces".
Olaf
On 8/3/23 19:53, Olaf Hering wrote:
> Thu, 3 Aug 2023 12:36:08 +0200 Michal Privoznik :
>
>> +vshControl _ctl = { 0 };
>
> I see this often, instead of a simple 'type variable = {};',
> and wonder what that zero is doing here?
That's C23 standard:
Thu, 3 Aug 2023 12:36:08 +0200 Michal Privoznik :
> +vshControl _ctl = { 0 };
I see this often, instead of a simple 'type variable = {};',
and wonder what that zero is doing here?
Olaf
pgpesWu2O0JH7.pgp
Description: Digitale Signatur von OpenPGP
On 8/3/23 13:29, Claudio Fontana wrote:
> On 8/3/23 12:36, Michal Privoznik wrote:
>> Inside of securityselinuxhelper we still use malloc() +
>> memset(.., 0, ...) combo. Convert it to g_new0().
>>
>> Signed-off-by: Michal Privoznik
>
> I don't think it is a good idea to mix Glib g_new, g_free
On 8/3/23 05:52, Martin Kletzander wrote:
On Wed, Aug 02, 2023 at 01:40:16PM -0600, Jim Fehlig wrote:
On 8/1/23 08:11, Martin Kletzander wrote:
On Mon, Jul 31, 2023 at 05:06:44PM -0600, Jim Fehlig wrote:
The first patch is trivial. I suppose the second is debatable. If I build
libvirt with
On 8/3/23 12:36, Michal Privoznik wrote:
> When a VSERPORT_CHANGE event is processed, we firstly do a little
> detour and try to detect whether the event is coming from guest
> agent. If so, we notify threads that are currently talking to the
> agent about this fact. Then we proceed with usual
On 8/3/23 12:36, Michal Privoznik wrote:
> If one of previous commits taught us something, it's that:
> sizeof(variable) and sizeof(type) are not the same. Especially
> because for live enough code the type might change (e.g. as we
> use autoptr more). And since we don't get any warnings when an
>
On 8/3/23 12:36, Michal Privoznik wrote:
> There are some cases left after previous commit which were not
> picked up by coccinelle. Mostly, becuase the spatch was not
> generic enough. We are left with cases like: two variables
> declared on one line, a variable declared in #ifdef-s (there are
>
On 8/3/23 12:36, Michal Privoznik wrote:
> This is a more concise approach and guarantees there is
> no time window where the struct is uninitialized.
>
> Generated using the following semantic patch:
>
> @@
> type T;
> identifier X;
> @@
> - T X;
> + T X = { 0 };
> ... when
On Wed, Aug 02, 2023 at 01:40:16PM -0600, Jim Fehlig wrote:
On 8/1/23 08:11, Martin Kletzander wrote:
On Mon, Jul 31, 2023 at 05:06:44PM -0600, Jim Fehlig wrote:
The first patch is trivial. I suppose the second is debatable. If I build
libvirt with -Dremote_default_mode=legacy but deploy
On 8/3/23 12:36, Michal Privoznik wrote:
> Ideally, these would be fixed by coccinelle (see next commit),
> but because of various reasons they aren't. Fix them manually.
>
> Signed-off-by: Michal Privoznik
Reviewed-by: Claudio Fontana
> ---
> src/rpc/virnetdaemon.c | 7 ++-
> 1 file
On 8/3/23 12:36, Michal Privoznik wrote:
> Instead of suggesting to zero structs out using memset() we
> should suggest initializing structs with zero initializer.
>
> Signed-off-by: Michal Privoznik
Reviewed-by: Claudio Fontana
> ---
> src/rpc/virnetclient.c | 4 ++--
> 1 file changed, 2
On 8/3/23 12:36, Michal Privoznik wrote:
> Inside of securityselinuxhelper we still use malloc() +
> memset(.., 0, ...) combo. Convert it to g_new0().
>
> Signed-off-by: Michal Privoznik
I don't think it is a good idea to mix Glib g_new, g_free etc with malloc,
calloc, free.
If you go with
On 8/3/23 12:36, Michal Privoznik wrote:
> The fds variable inside of virNetlinkCommand() is not used
> really. It's passed to memset() (hence compilers do not
> complain), but that's about it. Drop it.
>
> Signed-off-by: Michal Privoznik
Reviewed-by: Claudio Fontana
> ---
>
On 8/3/23 12:36, Michal Privoznik wrote:
> This is a residue of v6.8.0-rc1~100. The error variable inside of
> virFirewallDApplyRule() is already initialized to NULL. There's
> no need to memset() it to zero again.
>
> Signed-off-by: Michal Privoznik
Reviewed-by: Claudio Fontana
> ---
>
On 8/3/23 12:36, Michal Privoznik wrote:
> Inside of remoteAuthSASL() the sargs variable is already
> initialized to zero during declaration. There's no need to
> memset() it again as it's unused in between it's declaration and
> said memset().
>
> Signed-off-by: Michal Privoznik
Reviewed-by:
On 8/3/23 12:36, Michal Privoznik wrote:
> This is similar to the previous commit, except this is for a
> different type (vahControl) and also fixes the case where _ctl is
> passed not initialized to vah_error() (via ctl pointer so that's
> probably why compilers don't complain).
>
>
On 8/3/23 12:36, Michal Privoznik wrote:
> Both virsh and virt-admin have vshControl typed variables and
> also pointers to these variables. In both cases these are
> declared on a single line. Do the following:
>
> 1) break declaration into two lines,
> 2) use struct zero initializer for
On 8/3/23 12:36, Michal Privoznik wrote:
> There are couple of variables that are declared at function
> beginning but then used solely within a block (either for() loop
> or if() statement). And just before their use they are zeroed
> explicitly using memset(). Decrease their scope, use struct
Inside of securityselinuxhelper we still use malloc() +
memset(.., 0, ...) combo. Convert it to g_new0().
Signed-off-by: Michal Privoznik
---
tests/securityselinuxhelper.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tests/securityselinuxhelper.c
This is similar to the previous commit, except this is for a
different type (vahControl) and also fixes the case where _ctl is
passed not initialized to vah_error() (via ctl pointer so that's
probably why compilers don't complain).
Signed-off-by: Michal Privoznik
---
If one of previous commits taught us something, it's that:
sizeof(variable) and sizeof(type) are not the same. Especially
because for live enough code the type might change (e.g. as we
use autoptr more). And since we don't get any warnings when an
incorrect length is passed to memset() it is easy
Ideally, these would be fixed by coccinelle (see next commit),
but because of various reasons they aren't. Fix them manually.
Signed-off-by: Michal Privoznik
---
src/rpc/virnetdaemon.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/rpc/virnetdaemon.c
This is a more concise approach and guarantees there is
no time window where the struct is uninitialized.
Generated using the following semantic patch:
@@
type T;
identifier X;
@@
- T X;
+ T X = { 0 };
... when exists
(
- memset(, 0, sizeof(X));
|
- memset(, 0,
Instead of suggesting to zero structs out using memset() we
should suggest initializing structs with zero initializer.
Signed-off-by: Michal Privoznik
---
src/rpc/virnetclient.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetclient.c
There are some cases left after previous commit which were not
picked up by coccinelle. Mostly, becuase the spatch was not
generic enough. We are left with cases like: two variables
declared on one line, a variable declared in #ifdef-s (there are
notoriously difficult for coccinelle), arrays,
The fds variable inside of virNetlinkCommand() is not used
really. It's passed to memset() (hence compilers do not
complain), but that's about it. Drop it.
Signed-off-by: Michal Privoznik
---
src/util/virnetlink.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/util/virnetlink.c
Inside of remoteAuthSASL() the sargs variable is already
initialized to zero during declaration. There's no need to
memset() it again as it's unused in between it's declaration and
said memset().
Signed-off-by: Michal Privoznik
---
src/remote/remote_driver.c | 1 -
1 file changed, 1 deletion(-)
This is a residue of v6.8.0-rc1~100. The error variable inside of
virFirewallDApplyRule() is already initialized to NULL. There's
no need to memset() it to zero again.
Signed-off-by: Michal Privoznik
---
src/util/virfirewalld.c | 2 --
1 file changed, 2 deletions(-)
diff --git
There are couple of variables that are declared at function
beginning but then used solely within a block (either for() loop
or if() statement). And just before their use they are zeroed
explicitly using memset(). Decrease their scope, use struct zero
initializer and drop explicit memset().
When a VSERPORT_CHANGE event is processed, we firstly do a little
detour and try to detect whether the event is coming from guest
agent. If so, we notify threads that are currently talking to the
agent about this fact. Then we proceed with usual event
processing (BeginJob(), update domain def,
Both virsh and virt-admin have vshControl typed variables and
also pointers to these variables. In both cases these are
declared on a single line. Do the following:
1) break declaration into two lines,
2) use struct zero initializer for vshControl and
virshControl/vshAdmControl structs,
This was inspired by Martin's comment here:
https://listman.redhat.com/archives/libvir-list/2023-July/241007.html
It has sent me down a rabbit hole. But hey, it helped me to identify
some needless memset()-s, unused variables, problematic code patterns.
Michal Prívozník (13):
tools: Fix
On Wed, Aug 2, 2023 at 10:33 PM Jonathon Jongsma wrote:
> On 7/24/23 8:05 AM, Peter Krempa wrote:
[...]
> >
> > I've also noticed that using 'qcow2' format for the device doesn't work:
> >
> > error: internal error: process exited while connecting to monitor:
> > 2023-07-24T12:54:15.818631Z
34 matches
Mail list logo
