* [PATCH v2] qga-win: Free GMatchInfo properly
@ 2021-06-10 15:58 Kostiantyn Kostiuk
2021-06-10 16:02 ` Daniel P. Berrangé
2021-06-10 16:04 ` [PATCH " Philippe Mathieu-Daudé
0 siblings, 2 replies; 8+ messages in thread
From: Kostiantyn Kostiuk @ 2021-06-10 15:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth
The g_regex_match function creates match_info even if it
returns FALSE. So we should always call g_match_info_free.
A better solution is using g_autoptr for match_info variable.
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
---
qga/commands-win32.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 300b87c859..785a5cc6b2 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2494,7 +2494,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
continue;
}
for (j = 0; hw_ids[j] != NULL; j++) {
- GMatchInfo *match_info;
+ g_autoptr(GMatchInfo) match_info;
GuestDeviceIdPCI *id;
if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
continue;
@@ -2511,7 +2511,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
id->device_id = g_ascii_strtoull(device_id, NULL, 16);
- g_match_info_free(match_info);
break;
}
if (skip) {
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] qga-win: Free GMatchInfo properly
2021-06-10 15:58 [PATCH v2] qga-win: Free GMatchInfo properly Kostiantyn Kostiuk
@ 2021-06-10 16:02 ` Daniel P. Berrangé
2021-06-10 16:08 ` Konstantin Kostiuk
2021-06-10 16:04 ` [PATCH " Philippe Mathieu-Daudé
1 sibling, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-06-10 16:02 UTC (permalink / raw)
To: Kostiantyn Kostiuk; +Cc: qemu-devel, Michael Roth
On Thu, Jun 10, 2021 at 06:58:11PM +0300, Kostiantyn Kostiuk wrote:
> The g_regex_match function creates match_info even if it
> returns FALSE. So we should always call g_match_info_free.
> A better solution is using g_autoptr for match_info variable.
>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
> ---
> qga/commands-win32.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 300b87c859..785a5cc6b2 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -2494,7 +2494,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> continue;
> }
> for (j = 0; hw_ids[j] != NULL; j++) {
> - GMatchInfo *match_info;
> + g_autoptr(GMatchInfo) match_info;
This should be initialized to NULL otherwise...
> GuestDeviceIdPCI *id;
> if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
> continue;
this continue will trigger freeing of unintialized memory
Essentially all g_auto* variables should be init to NULL
at all times, even if it currently looks harmless.
> @@ -2511,7 +2511,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> id->device_id = g_ascii_strtoull(device_id, NULL, 16);
>
> - g_match_info_free(match_info);
> break;
> }
> if (skip) {
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] qga-win: Free GMatchInfo properly
2021-06-10 16:02 ` Daniel P. Berrangé
@ 2021-06-10 16:08 ` Konstantin Kostiuk
2021-06-10 16:14 ` Daniel P. Berrangé
0 siblings, 1 reply; 8+ messages in thread
From: Konstantin Kostiuk @ 2021-06-10 16:08 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: Developers, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]
On Thu, Jun 10, 2021 at 7:02 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> On Thu, Jun 10, 2021 at 06:58:11PM +0300, Kostiantyn Kostiuk wrote:
> > The g_regex_match function creates match_info even if it
> > returns FALSE. So we should always call g_match_info_free.
> > A better solution is using g_autoptr for match_info variable.
> >
> > Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
> > ---
> > qga/commands-win32.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 300b87c859..785a5cc6b2 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -2494,7 +2494,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error
> **errp)
> > continue;
> > }
> > for (j = 0; hw_ids[j] != NULL; j++) {
> > - GMatchInfo *match_info;
> > + g_autoptr(GMatchInfo) match_info;
>
> This should be initialized to NULL otherwise...
>
> > GuestDeviceIdPCI *id;
> > if (!g_regex_match(device_pci_re, hw_ids[j], 0,
> &match_info)) {
> > continue;
>
> this continue will trigger freeing of unintialized memory
>
But we always call match_info, so match_info is always initialized.
The g_regex_match function creates match_info even if it returns FALSE.
>
> Essentially all g_auto* variables should be init to NULL
> at all times, even if it currently looks harmless.
>
> > @@ -2511,7 +2511,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error
> **errp)
> > id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
> > id->device_id = g_ascii_strtoull(device_id, NULL, 16);
> >
> > - g_match_info_free(match_info);
> > break;
> > }
> > if (skip) {
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o-
> https://www.instagram.com/dberrange :|
>
>
Best wishes,
Kostiantyn Kostiuk
[-- Attachment #2: Type: text/html, Size: 3641 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] qga-win: Free GMatchInfo properly
2021-06-10 16:08 ` Konstantin Kostiuk
@ 2021-06-10 16:14 ` Daniel P. Berrangé
2021-07-14 7:26 ` Konstantin Kostiuk
0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-06-10 16:14 UTC (permalink / raw)
To: Konstantin Kostiuk; +Cc: Developers, Michael Roth
On Thu, Jun 10, 2021 at 07:08:36PM +0300, Konstantin Kostiuk wrote:
> On Thu, Jun 10, 2021 at 7:02 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
>
> > On Thu, Jun 10, 2021 at 06:58:11PM +0300, Kostiantyn Kostiuk wrote:
> > > The g_regex_match function creates match_info even if it
> > > returns FALSE. So we should always call g_match_info_free.
> > > A better solution is using g_autoptr for match_info variable.
> > >
> > > Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
> > > ---
> > > qga/commands-win32.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > > index 300b87c859..785a5cc6b2 100644
> > > --- a/qga/commands-win32.c
> > > +++ b/qga/commands-win32.c
> > > @@ -2494,7 +2494,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error
> > **errp)
> > > continue;
> > > }
> > > for (j = 0; hw_ids[j] != NULL; j++) {
> > > - GMatchInfo *match_info;
> > > + g_autoptr(GMatchInfo) match_info;
> >
> > This should be initialized to NULL otherwise...
> >
> > > GuestDeviceIdPCI *id;
> > > if (!g_regex_match(device_pci_re, hw_ids[j], 0,
> > &match_info)) {
> > > continue;
> >
> > this continue will trigger freeing of unintialized memory
> >
>
> But we always call match_info, so match_info is always initialized.
> The g_regex_match function creates match_info even if it returns FALSE.
Opps, yes, you are right.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] qga-win: Free GMatchInfo properly
2021-06-10 16:14 ` Daniel P. Berrangé
@ 2021-07-14 7:26 ` Konstantin Kostiuk
2021-07-28 7:54 ` Konstantin Kostiuk
0 siblings, 1 reply; 8+ messages in thread
From: Konstantin Kostiuk @ 2021-07-14 7:26 UTC (permalink / raw)
To: Michael Roth; +Cc: Daniel P. Berrangé, Developers, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]
CC Michael Roth
On Thu, Jun 10, 2021 at 7:14 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> On Thu, Jun 10, 2021 at 07:08:36PM +0300, Konstantin Kostiuk wrote:
> > On Thu, Jun 10, 2021 at 7:02 PM Daniel P. Berrangé <berrange@redhat.com>
> > wrote:
> >
> > > On Thu, Jun 10, 2021 at 06:58:11PM +0300, Kostiantyn Kostiuk wrote:
> > > > The g_regex_match function creates match_info even if it
> > > > returns FALSE. So we should always call g_match_info_free.
> > > > A better solution is using g_autoptr for match_info variable.
> > > >
> > > > Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
> > > > ---
> > > > qga/commands-win32.c | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > > > index 300b87c859..785a5cc6b2 100644
> > > > --- a/qga/commands-win32.c
> > > > +++ b/qga/commands-win32.c
> > > > @@ -2494,7 +2494,7 @@ GuestDeviceInfoList
> *qmp_guest_get_devices(Error
> > > **errp)
> > > > continue;
> > > > }
> > > > for (j = 0; hw_ids[j] != NULL; j++) {
> > > > - GMatchInfo *match_info;
> > > > + g_autoptr(GMatchInfo) match_info;
> > >
> > > This should be initialized to NULL otherwise...
> > >
> > > > GuestDeviceIdPCI *id;
> > > > if (!g_regex_match(device_pci_re, hw_ids[j], 0,
> > > &match_info)) {
> > > > continue;
> > >
> > > this continue will trigger freeing of unintialized memory
> > >
> >
> > But we always call match_info, so match_info is always initialized.
> > The g_regex_match function creates match_info even if it returns FALSE.
>
> Opps, yes, you are right.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o-
> https://www.instagram.com/dberrange :|
>
>
[-- Attachment #2: Type: text/html, Size: 3468 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] qga-win: Free GMatchInfo properly
2021-07-14 7:26 ` Konstantin Kostiuk
@ 2021-07-28 7:54 ` Konstantin Kostiuk
2021-07-28 11:58 ` [PATCH-for-6.1 " Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Konstantin Kostiuk @ 2021-07-28 7:54 UTC (permalink / raw)
To: Michael Roth; +Cc: Daniel P. Berrangé, Developers, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]
ping
On Wed, Jul 14, 2021 at 10:26 AM Konstantin Kostiuk <konstantin@daynix.com>
wrote:
> CC Michael Roth
>
> On Thu, Jun 10, 2021 at 7:14 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
>
>> On Thu, Jun 10, 2021 at 07:08:36PM +0300, Konstantin Kostiuk wrote:
>> > On Thu, Jun 10, 2021 at 7:02 PM Daniel P. Berrangé <berrange@redhat.com
>> >
>> > wrote:
>> >
>> > > On Thu, Jun 10, 2021 at 06:58:11PM +0300, Kostiantyn Kostiuk wrote:
>> > > > The g_regex_match function creates match_info even if it
>> > > > returns FALSE. So we should always call g_match_info_free.
>> > > > A better solution is using g_autoptr for match_info variable.
>> > > >
>> > > > Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
>> > > > ---
>> > > > qga/commands-win32.c | 3 +--
>> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
>> > > >
>> > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> > > > index 300b87c859..785a5cc6b2 100644
>> > > > --- a/qga/commands-win32.c
>> > > > +++ b/qga/commands-win32.c
>> > > > @@ -2494,7 +2494,7 @@ GuestDeviceInfoList
>> *qmp_guest_get_devices(Error
>> > > **errp)
>> > > > continue;
>> > > > }
>> > > > for (j = 0; hw_ids[j] != NULL; j++) {
>> > > > - GMatchInfo *match_info;
>> > > > + g_autoptr(GMatchInfo) match_info;
>> > >
>> > > This should be initialized to NULL otherwise...
>> > >
>> > > > GuestDeviceIdPCI *id;
>> > > > if (!g_regex_match(device_pci_re, hw_ids[j], 0,
>> > > &match_info)) {
>> > > > continue;
>> > >
>> > > this continue will trigger freeing of unintialized memory
>> > >
>> >
>> > But we always call match_info, so match_info is always initialized.
>> > The g_regex_match function creates match_info even if it returns FALSE.
>>
>> Opps, yes, you are right.
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>>
>> Regards,
>> Daniel
>> --
>> |: https://berrange.com -o-
>> https://www.flickr.com/photos/dberrange :|
>> |: https://libvirt.org -o-
>> https://fstop138.berrange.com :|
>> |: https://entangle-photo.org -o-
>> https://www.instagram.com/dberrange :|
>>
>>
[-- Attachment #2: Type: text/html, Size: 3859 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH-for-6.1 v2] qga-win: Free GMatchInfo properly
2021-07-28 7:54 ` Konstantin Kostiuk
@ 2021-07-28 11:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-28 11:58 UTC (permalink / raw)
To: Konstantin Kostiuk, Michael Roth
Cc: Daniel P. Berrangé, Developers, Michael Roth
Still candidate for 6.1.
On 7/28/21 9:54 AM, Konstantin Kostiuk wrote:
> ping
>
> On Wed, Jul 14, 2021 at 10:26 AM Konstantin Kostiuk
> <konstantin@daynix.com <mailto:konstantin@daynix.com>> wrote:
>
> CC Michael Roth
>
> On Thu, Jun 10, 2021 at 7:14 PM Daniel P. Berrangé
> <berrange@redhat.com <mailto:berrange@redhat.com>> wrote:
>
> On Thu, Jun 10, 2021 at 07:08:36PM +0300, Konstantin Kostiuk wrote:
> > On Thu, Jun 10, 2021 at 7:02 PM Daniel P. Berrangé
> <berrange@redhat.com <mailto:berrange@redhat.com>>
> > wrote:
> >
> > > On Thu, Jun 10, 2021 at 06:58:11PM +0300, Kostiantyn Kostiuk
> wrote:
> > > > The g_regex_match function creates match_info even if it
> > > > returns FALSE. So we should always call g_match_info_free.
> > > > A better solution is using g_autoptr for match_info variable.
> > > >
> > > > Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com
> <mailto:konstantin@daynix.com>>
> > > > ---
> > > > qga/commands-win32.c | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > > > index 300b87c859..785a5cc6b2 100644
> > > > --- a/qga/commands-win32.c
> > > > +++ b/qga/commands-win32.c
> > > > @@ -2494,7 +2494,7 @@ GuestDeviceInfoList
> *qmp_guest_get_devices(Error
> > > **errp)
> > > > continue;
> > > > }
> > > > for (j = 0; hw_ids[j] != NULL; j++) {
> > > > - GMatchInfo *match_info;
> > > > + g_autoptr(GMatchInfo) match_info;
> > >
> > > This should be initialized to NULL otherwise...
> > >
> > > > GuestDeviceIdPCI *id;
> > > > if (!g_regex_match(device_pci_re, hw_ids[j], 0,
> > > &match_info)) {
> > > > continue;
> > >
> > > this continue will trigger freeing of unintialized memory
> > >
> >
> > But we always call match_info, so match_info is always
> initialized.
> > The g_regex_match function creates match_info even if it
> returns FALSE.
>
> Opps, yes, you are right.
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com
> <mailto:berrange@redhat.com>>
>
>
> Regards,
> Daniel
> --
> |: https://berrange.com <https://berrange.com> -o-
> https://www.flickr.com/photos/dberrange
> <https://www.flickr.com/photos/dberrange> :|
> |: https://libvirt.org <https://libvirt.org> -o-
> https://fstop138.berrange.com <https://fstop138.berrange.com> :|
> |: https://entangle-photo.org <https://entangle-photo.org>
> -o- https://www.instagram.com/dberrange
> <https://www.instagram.com/dberrange> :|
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] qga-win: Free GMatchInfo properly
2021-06-10 15:58 [PATCH v2] qga-win: Free GMatchInfo properly Kostiantyn Kostiuk
2021-06-10 16:02 ` Daniel P. Berrangé
@ 2021-06-10 16:04 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-10 16:04 UTC (permalink / raw)
To: Kostiantyn Kostiuk, qemu-devel; +Cc: Michael Roth
On 6/10/21 5:58 PM, Kostiantyn Kostiuk wrote:
> The g_regex_match function creates match_info even if it
> returns FALSE. So we should always call g_match_info_free.
> A better solution is using g_autoptr for match_info variable.
>
> Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
> ---
> qga/commands-win32.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-28 12:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10 15:58 [PATCH v2] qga-win: Free GMatchInfo properly Kostiantyn Kostiuk
2021-06-10 16:02 ` Daniel P. Berrangé
2021-06-10 16:08 ` Konstantin Kostiuk
2021-06-10 16:14 ` Daniel P. Berrangé
2021-07-14 7:26 ` Konstantin Kostiuk
2021-07-28 7:54 ` Konstantin Kostiuk
2021-07-28 11:58 ` [PATCH-for-6.1 " Philippe Mathieu-Daudé
2021-06-10 16:04 ` [PATCH " Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.