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

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 5476 bytes --]