Re: [PATCH for-11.0 2/6] Revert "error: define g_autoptr() cleanup function for the Error type"

2025-11-26 Thread Maciej S. Szmigiero

On 25.11.2025 21:46, Peter Xu wrote:

This reverts commit 18eb55546a54e443d94a4c49286348176ad4b00a.  Discussion
can be seen at:

https://lore.kernel.org/r/[email protected]

Cc: Maciej S. Szmigiero 
Cc: Cédric Le Goater 
Signed-off-by: Peter Xu 
---
  include/qapi/error.h | 2 --
  1 file changed, 2 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index b16c6303f8..f3ce4a4a2d 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -437,8 +437,6 @@ Error *error_copy(const Error *err);
   */
  void error_free(Error *err);
  
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free)

-
  /*
   * Convenience function to assert that *@errp is set, then silently free it.
   */


Acked-by: Maciej S. Szmigiero 

Thanks,
Maciej




Re: [PATCH for-11.0 2/6] Revert "error: define g_autoptr() cleanup function for the Error type"

2025-11-26 Thread Markus Armbruster
Cédric Le Goater  writes:

> On 11/25/25 21:46, Peter Xu wrote:
>> This reverts commit 18eb55546a54e443d94a4c49286348176ad4b00a.  Discussion
>> can be seen at:
>> https://lore.kernel.org/r/[email protected]
>> Cc: Maciej S. Szmigiero 
>> Cc: Cédric Le Goater 
>> Signed-off-by: Peter Xu 
>> ---
>>  include/qapi/error.h | 2 --
>>  1 file changed, 2 deletions(-)
>> diff --git a/include/qapi/error.h b/include/qapi/error.h
>> index b16c6303f8..f3ce4a4a2d 100644
>> --- a/include/qapi/error.h
>> +++ b/include/qapi/error.h
>> @@ -437,8 +437,6 @@ Error *error_copy(const Error *err);
>>   */
>>  void error_free(Error *err);
>>
>> -G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free)
>> -
>>  /*
>>   * Convenience function to assert that *@errp is set, then silently free it.
>>   */
>
> Is that related to CID 1643463 issue ?

g_autoptr(Error) is a bad idea, as discussed in

Subject: g_autoptr(Error) (was: [PATCH] migration: Fix double-free on error 
path) 
Date: Tue, 25 Nov 2025 08:40:07 +0100
Message-ID: <[email protected]>

We have three instances of g_autoptr(Error) in master.  CID 1643463 made
me see them.

One is removed by my fix to CID 1643463:

Subject: [PATCH] migration: Fix double-free on error path
Date: Tue, 25 Nov 2025 08:05:54 +0100
Message-ID: <[email protected]>

The remaining two get removed in PATCH 1.  This patch deletes the code
that makes g_autoptr(Error) work.

> anyhow,
>
>
> Reviewed-by: Cédric Le Goater 

Thank you!




Re: [PATCH for-11.0 2/6] Revert "error: define g_autoptr() cleanup function for the Error type"

2025-11-25 Thread Cédric Le Goater

On 11/25/25 21:46, Peter Xu wrote:

This reverts commit 18eb55546a54e443d94a4c49286348176ad4b00a.  Discussion
can be seen at:

https://lore.kernel.org/r/[email protected]

Cc: Maciej S. Szmigiero 
Cc: Cédric Le Goater 
Signed-off-by: Peter Xu 
---
  include/qapi/error.h | 2 --
  1 file changed, 2 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index b16c6303f8..f3ce4a4a2d 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -437,8 +437,6 @@ Error *error_copy(const Error *err);
   */
  void error_free(Error *err);
  
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free)

-
  /*
   * Convenience function to assert that *@errp is set, then silently free it.
   */


Is that related to CID 1643463 issue ?

anyhow,


Reviewed-by: Cédric Le Goater 

Thanks,

C.





Re: [PATCH for-11.0 2/6] Revert "error: define g_autoptr() cleanup function for the Error type"

2025-11-25 Thread Markus Armbruster
Peter Xu  writes:

> This reverts commit 18eb55546a54e443d94a4c49286348176ad4b00a.  Discussion
> can be seen at:
>
> https://lore.kernel.org/r/[email protected]

Suggest to steal from the previous commit like this:

  Due to the nature of how Error should be used (normally ownership will be
  passed over to Error APIs, like error_report_err), auto-free functions may
  be error prone on its own.  The auto cleanup function was merged without 
  proper review as pointed out by Dan and Markus:

  https://lore.kernel.org/r/[email protected]

> Cc: Maciej S. Szmigiero 
> Cc: Cédric Le Goater 
> Signed-off-by: Peter Xu 
> ---
>  include/qapi/error.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index b16c6303f8..f3ce4a4a2d 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -437,8 +437,6 @@ Error *error_copy(const Error *err);
>   */
>  void error_free(Error *err);
>  
> -G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free)
> -
>  /*
>   * Convenience function to assert that *@errp is set, then silently free it.
>   */

Reviewed-by: Markus Armbruster