On 12/19/25 3:16 PM, Jan Beulich wrote:
On 19.12.2025 14:32, Oleksii Kurochko wrote:
On 12/18/25 7:15 PM, Andrew Cooper wrote:
int vcpu_vgic_free(struct vcpu *v)
{
- xfree(v->arch.vgic.private_irqs);
+ xfree(v->arch.vgic->private_irqs);
+ xfree(v->arch.vgic);
+
return 0;
}
Free functions should be idempotent. This was buggy before, even
moreso
now.
Was it really buggy before in terms of idempotent.
It seems like xfree() can handle the case when
v->arch.vgic.private_irqs is NULL.
Should I still have:
if ( v->arch.vgic->private_irqs )
XFREE(v->arch.vgic->private_irqs);
?
No, and iirc Andrew also didn't ask for this.
Andrew wrote:
```
Free functions should be idempotent. This was buggy before, even moreso
now. It wants to be:
void vcpu_vgic_free(struct vcpu *v)
{
if ( v->arch.vgic )
{
XFREE(v->arch.vgic->private_irqs);
XFREE(v->arch.vgic);
}
}
Given the type change, this probably wants splitting out into an earlier
patch.
Given the fact that the single caller doesn't even check the return
value, you're fixing a MISRA violation by making it void.
```
It isn't clear what "This was buggy before" means. In my changes
it will be an issue if vcpu_vgic_free() would be called twice because
without "if ( v->arch.vgic )" we will have NULL pointer dereference
XFREE(v->arch.vgic->private_irqs) here. But considering that before
it was just:
int vcpu_vgic_free(struct vcpu *v)
{
xfree(v->arch.vgic.private_irqs);
return 0;
}
In that case, it seems that calling|vcpu_vgic_free()| twice would not
cause
anything serious to happen. Am I misunderstanding what was buggy in
the original
code, or probably I'm misunderstanding what is idempotent?
