| [Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
 Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense
 
To: Markus Armbruster <armbru@xxxxxxxxxx>From: Alex Bennée <alex.bennee@xxxxxxxxxx>Date: Tue, 15 Mar 2022 16:16:54 +0000Cc: Philippe Mathieu-Daudé <philippe.mathieu.daude@xxxxxxxxx>, qemu-devel@xxxxxxxxxx, Paolo  Bonzini <pbonzini@xxxxxxxxxx>, Richard Henderson <richard.henderson@xxxxxxxxxx>, Gerd Hoffmann <kraxel@xxxxxxxxxx>, Christian Schoenebeck <qemu_oss@xxxxxxxxxxxxx>, "Gonglei (Arei)" <arei.gonglei@xxxxxxxxxx>, Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>, "Michael S. Tsirkin" <mst@xxxxxxxxxx>, Igor Mammedov <imammedo@xxxxxxxxxx>, Ani Sinha <ani@xxxxxxxxxxx>, Laurent Vivier <lvivier@xxxxxxxxxx>, Amit Shah <amit@xxxxxxxxxx>, Peter  Maydell <peter.maydell@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Hervé Poussineau <hpoussin@xxxxxxxxxxx>, Aleksandar Rikalo <aleksandar.rikalo@xxxxxxxxxx>, Corey Minyard <cminyard@xxxxxxxxxx>, Patrick Venture <venture@xxxxxxxxxx>, Eduardo  Habkost <eduardo@xxxxxxxxxxx>, Marcel Apfelbaum <marcel.apfelbaum@xxxxxxxxx>, Peter Xu <peterx@xxxxxxxxxx>, Jason Wang <jasowang@xxxxxxxxxx>, Cédric Le Goater <clg@xxxxxxxx>, Daniel Henrique  Barboza <danielhb413@xxxxxxxxx>, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>, Greg Kurz <groug@xxxxxxxx>, Philippe  Mathieu-Daudé <f4bug@xxxxxxxxx>, Jean-Christophe Dubois <jcd@xxxxxxxxxxxxxxx>, Keith Busch <kbusch@xxxxxxxxxx>, Klaus Jensen <its@xxxxxxxxxxxxx>, Yuval Shaia <yuval.shaia.ml@xxxxxxxxx>, Yoshinori  Sato <ysato@xxxxxxxxxxxxxxxxxxxx>, Magnus Damm <magnus.damm@xxxxxxxxx>, Fabien Chouteau <chouteau@xxxxxxxxxxx>, KONRAD Frederic <frederic.konrad@xxxxxxxxxxx>, Mark Cave-Ayland <mark.cave-ayland@xxxxxxxxxxxx>, Artyom Tarasenko <atar4qemu@xxxxxxxxx>, Alex Williamson <alex.williamson@xxxxxxxxxx>, Eric Auger <eric.auger@xxxxxxxxxx>, Max Filippov <jcmvbkbc@xxxxxxxxx>, Juan  Quintela <quintela@xxxxxxxxxx>, "Dr. David Alan Gilbert" <dgilbert@xxxxxxxxxx>, Konstantin Kostiuk <kkostiuk@xxxxxxxxxx>, Michael  Roth <michael.roth@xxxxxxx>, Daniel P. Berrangé <berrange@xxxxxxxxxx>, Pavel Dovgalyuk <pavel.dovgaluk@xxxxxxxxx>, David Hildenbrand <david@xxxxxxxxxx>, Wenchao Wang <wenchao.wang@xxxxxxxxx>, Colin Xu <colin.xu@xxxxxxxxx>, Kamil Rytarowski <kamil@xxxxxxxxxx>, Reinoud  Zandijk <reinoud@xxxxxxxxxx>, Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>, Cornelia Huck <cohuck@xxxxxxxxxx>, Thomas Huth <thuth@xxxxxxxxxx>, Eric  Blake <eblake@xxxxxxxxxx>, Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx>, John Snow <jsnow@xxxxxxxxxx>, kvm@xxxxxxxxxxxxxxx, qemu-arm@xxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx, qemu-ppc@xxxxxxxxxx, qemu-block@xxxxxxxxxx, haxm-team@xxxxxxxxx, qemu-s390x@xxxxxxxxxxDelivery-date: Tue, 15 Mar 2022 16:17:50 +0000List-id: Xen developer discussion <xen-devel.lists.xenproject.org> 
 
Markus Armbruster <armbru@xxxxxxxxxx> writes:
> Philippe Mathieu-Daudé <philippe.mathieu.daude@xxxxxxxxx> writes:
>
>> On 15/3/22 14:59, Markus Armbruster wrote:
>>> Alex Bennée <alex.bennee@xxxxxxxxxx> writes:
>>> 
>>>> Markus Armbruster <armbru@xxxxxxxxxx> writes:
>>>>
>>>>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>>>>> for two reasons.  One, it catches multiplication overflowing size_t.
>>>>> Two, it returns T * rather than void *, which lets the compiler catch
>>>>> more type errors.
>>>>>
>>>> <snip>
>>>>> diff --git a/semihosting/config.c b/semihosting/config.c
>>>>> index 137171b717..6d48ec9566 100644
>>>>> --- a/semihosting/config.c
>>>>> +++ b/semihosting/config.c
>>>>> @@ -98,7 +98,7 @@ static int add_semihosting_arg(void *opaque,
>>>>>       if (strcmp(name, "arg") == 0) {
>>>>>           s->argc++;
>>>>>           /* one extra element as g_strjoinv() expects NULL-terminated 
>>>>> array */
>>>>> -        s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
>>>>> +        s->argv = g_renew(void *, s->argv, s->argc + 1);
>>>>
>>>> This did indeed break CI because s->argv is an array of *char:
>>>>
>>>> ../semihosting/config.c:101:17: error: assignment to ‘const char **’ from 
>>>> incompatible pointer type ‘void **’ [-Werror=incompatible-pointer-types]
>>>>    101 |         s->argv = g_renew(void *, s->argv, s->argc + 1);
>>>>        |                 ^
>>>> cc1: all warnings being treated as errors
>>>>
>>>> So it did the job of type checking but failed to build ;-)
>>>
>>> You found a hole in my compile testing, thanks!
>>>
>>> I got confused about the configuration of my build trees.  Catching such
>>> mistakes is what CI is for :)
>>
>> FYI Alex fixed this here:
>> https://lore.kernel.org/qemu-devel/20220315121251.2280317-8-alex.bennee@xxxxxxxxxx/
>>
>> So your series could go on top (modulo the Coverity change).
>
> I dropped this hunk in v2.
>
> Whether my v2 or Alex's series goes in first doesn't matter.
That's great. Thanks for finding the ugliness in the first place ;-)
-- 
Alex Bennée
 |