qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Byte Sized task (was Re: [PATCH] linux-user/syscall.c: malloc to g_t


From: John Snow
Subject: Re: Byte Sized task (was Re: [PATCH] linux-user/syscall.c: malloc to g_try_malloc)
Date: Mon, 10 Jan 2022 14:42:45 -0500



On Thu, Jan 6, 2022 at 9:10 AM Laurent Vivier <laurent@vivier.eu> wrote:
Hi Ahmed,

the email address you use for the trivial ML is broken. I've fixed it for my answer and added
qemu-devel.

Le 05/01/2022 à 20:09, Ahmed Abouzied a écrit :
> Hello,
>
> Thanks for the review. I also found some occurrences of "cpu_physical_memor_*" calls that should be
> replaced with "address_space_*" around the codebase as mentioned
> [here](https://wiki.qemu.org/Contribute/BiteSizedTasks
> <https://wiki.qemu.org/Contribute/BiteSizedTasks>) in the API conversion section. The "/hw"
> directory for instance. Should I go about replacing those as well? Should I create a Gitlab issue
> first to track the changes?


According to the wiki, the gitlab issue are already created with the "Byte Sized" label.

https://gitlab.com/qemu-project/qemu/-/issues?scope=all&state=opened&label_name[]=Bite%20Sized

But this one is not, I think John can help you for that (added in cc:)

Thanks,
Laurent

> Best regards,
> Ahmed
>
> On Wed, 5 Jan 2022 at 12:26, Laurent Vivier <laurent@vivier.eu <mailto:laurent@vivier.eu>> wrote:
>
>     Le 04/01/2022 à 15:38, Ahmed Abouzied a écrit :
>      > Use g_try_malloc instead of malloc to alocate the target ifconfig.
>      > Also replace the corresponding free with g_free.
>      >
>      > Signed-off-by: Ahmed Abouzied <email@aabouzied.com <mailto:email@aabouzied.com>>
>      > ---
>      >
>      > Hello,
>      >
>      > I noticed that there was a `malloc` call in this file. It seems that it
>      > was added by the commit 22e4a267 (3 years ago) which was after the commit
>      > 0e173b24 (6 years ago) that replaced malloc calls with glib alternative calls.
>      >
>      > There is no issue for this on Gitlab. Should I have created an issue
>      > first?
>      >
>      > Best regards,
>      >
>      >   linux-user/syscall.c | 4 ++--
>      >   1 file changed, 2 insertions(+), 2 deletions(-)
>      >
>      > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>      > index 56a3e17183..715f9430e1 100644
>      > --- a/linux-user/syscall.c
>      > +++ b/linux-user/syscall.c
>      > @@ -4867,7 +4867,7 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
>      >                * We can't fit all the extents into the fixed size buffer.
>      >                * Allocate one that is large enough and use it instead.
>      >                */
>      > -            host_ifconf = malloc(outbufsz);
>      > +            host_ifconf = g_try_malloc(outbufsz);
>      >               if (!host_ifconf) {
>      >                   return -TARGET_ENOMEM;
>      >               }
>      > @@ -4915,7 +4915,7 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp,
>      >       }
>      >
>      >       if (free_buf) {
>      > -        free(host_ifconf);
>      > +        g_free(host_ifconf);
>      >       }
>      >
>      >       return ret;
>
>     Applied to my linux-user-for-7.0 branch.
>
>     Thanks,
>     Laurent
>


IIRC, consensus was that this task was not "bite sized", as it might possibly be a large task without an easily attainable "end". You don't need to worry about creating a GitLab issue to track it (Unless you WANT to for your own tracking, then go ahead and make one), but if you believe that the task is now *done* in its entirety, the task should be removed from the wiki.

Feel free to send me questions if you've got any.

--js

reply via email to

[Prev in Thread] Current Thread [Next in Thread]