Marc-André Lureau <[email protected]> writes: > Hi > > On Tue, Nov 25, 2025 at 11:06 AM Markus Armbruster <[email protected]> wrote: >> >> Fixes: ffaa1b50a879 (migration: Use warn_reportf_err() where appropriate) >> Resolves: Coverity CID 1643463 >> Signed-off-by: Markus Armbruster <[email protected]> > > Reviewed-by: Marc-André Lureau <[email protected]> > >> --- >> migration/multifd.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/migration/multifd.c b/migration/multifd.c >> index 6210454838..3203dc98e1 100644 >> --- a/migration/multifd.c >> +++ b/migration/multifd.c >> @@ -450,7 +450,7 @@ static void multifd_send_set_error(Error *err) >> */ >> static void migration_ioc_shutdown_gracefully(QIOChannel *ioc) >> { >> - g_autoptr(Error) local_err = NULL; >> + Error *local_err = NULL; >> >> if (!migration_has_failed(migrate_get_current()) && >> object_dynamic_cast((Object *)ioc, TYPE_QIO_CHANNEL_TLS)) { >> -- >> 2.49.0 >> > > Maybe warn_reportf_err() should take a Error **err instead, and clear > it (and accept NULL values)
Our deallocating functions don't work that way. Having them take a pointer by reference and clear it gets rid of *one* dangling reference. There may be more. Coverity is fairly good at finding the kind of use after free this could avoid. Error ** parameters are almost always for returning errors. Not having to wonder what such a parameter is for makes code easier to read. Thanks for your review!
