| Message ID | 1458757923-11362-1-git-send-email-haris.phnx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, Can I get a respond for this patch. Is it complete, or did I miss something? On Thu, Mar 24, 2016 at 12:02 AM, Md Haris Iqbal <haris.phnx@gmail.com> wrote: > Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> > --- > linux-user/qemu.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 26b0ba2..3c3fd15 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy > #ifdef DEBUG_REMAP > { > void *addr; > - addr = malloc(len); > + addr = g_malloc(len); > if (copy) > memcpy(addr, g2h(guest_addr), len); > else > @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, > return; > if (len > 0) > memcpy(g2h(guest_addr), host_ptr, len); > - free(host_ptr); > + g_free(host_ptr); > #endif > } > > -- > 1.9.1 >
Hi, Patch is attached in the previous mail. Looking forward to your review. On Tue, Mar 29, 2016 at 9:23 PM, haris iqbal <haris.phnx@gmail.com> wrote: > Hi, > > Can I get a respond for this patch. Is it complete, or did I miss something? > > > On Thu, Mar 24, 2016 at 12:02 AM, Md Haris Iqbal <haris.phnx@gmail.com> wrote: >> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> >> --- >> linux-user/qemu.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h >> index 26b0ba2..3c3fd15 100644 >> --- a/linux-user/qemu.h >> +++ b/linux-user/qemu.h >> @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy >> #ifdef DEBUG_REMAP >> { >> void *addr; >> - addr = malloc(len); >> + addr = g_malloc(len); >> if (copy) >> memcpy(addr, g2h(guest_addr), len); >> else >> @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, >> return; >> if (len > 0) >> memcpy(g2h(guest_addr), host_ptr, len); >> - free(host_ptr); >> + g_free(host_ptr); >> #endif >> } >> >> -- >> 1.9.1 >> > > > > -- > > With regards, > > Md Haris Iqbal, > Placement Coordinator, MTech IT > NITK Surathkal, > Contact: +91 8861996962
On Thu, Mar 24, 2016 at 12:02:03AM +0530, Md Haris Iqbal wrote: > Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> > --- > linux-user/qemu.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 26b0ba2..3c3fd15 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy > #ifdef DEBUG_REMAP > { > void *addr; > - addr = malloc(len); > + addr = g_malloc(len); > if (copy) > memcpy(addr, g2h(guest_addr), len); > else > @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, > return; > if (len > 0) > memcpy(g2h(guest_addr), host_ptr, len); > - free(host_ptr); > + g_free(host_ptr); > #endif > } If I understand correctly either lock_user() or lock_user_string() may be followed by unlock_user(). If you change unlock_user() to g_free() then you also need to change lock_user_string() to g_malloc() to avoid a malloc()/g_free() mismatch.
On Wed, Mar 30, 2016 at 7:39 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Thu, Mar 24, 2016 at 12:02:03AM +0530, Md Haris Iqbal wrote: >> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> >> --- >> linux-user/qemu.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h >> index 26b0ba2..3c3fd15 100644 >> --- a/linux-user/qemu.h >> +++ b/linux-user/qemu.h >> @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy >> #ifdef DEBUG_REMAP >> { >> void *addr; >> - addr = malloc(len); >> + addr = g_malloc(len); >> if (copy) >> memcpy(addr, g2h(guest_addr), len); >> else >> @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, >> return; >> if (len > 0) >> memcpy(g2h(guest_addr), host_ptr, len); >> - free(host_ptr); >> + g_free(host_ptr); >> #endif >> } > > If I understand correctly either lock_user() or lock_user_string() may > be followed by unlock_user(). If you change unlock_user() to g_free() > then you also need to change lock_user_string() to g_malloc() to avoid a > malloc()/g_free() mismatch. lock_user_string() does not use malloc itself, but calls lock_user() from itself.
On Wed, Mar 30, 2016 at 09:58:41PM +0530, haris iqbal wrote: > On Wed, Mar 30, 2016 at 7:39 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Thu, Mar 24, 2016 at 12:02:03AM +0530, Md Haris Iqbal wrote: > >> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> > >> --- > >> linux-user/qemu.h | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h > >> index 26b0ba2..3c3fd15 100644 > >> --- a/linux-user/qemu.h > >> +++ b/linux-user/qemu.h > >> @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy > >> #ifdef DEBUG_REMAP > >> { > >> void *addr; > >> - addr = malloc(len); > >> + addr = g_malloc(len); > >> if (copy) > >> memcpy(addr, g2h(guest_addr), len); > >> else > >> @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, > >> return; > >> if (len > 0) > >> memcpy(g2h(guest_addr), host_ptr, len); > >> - free(host_ptr); > >> + g_free(host_ptr); > >> #endif > >> } > > > > If I understand correctly either lock_user() or lock_user_string() may > > be followed by unlock_user(). If you change unlock_user() to g_free() > > then you also need to change lock_user_string() to g_malloc() to avoid a > > malloc()/g_free() mismatch. > > lock_user_string() does not use malloc itself, but calls lock_user() > from itself. You are right. The reason I got confused is because there are 3 versions of lock_user(): linux-user, bsd-user, and include/exec/softmmu-semi.h. Please send equivalent patches for the other 2 versions as well. This way they stay consistent. Thanks, Stefan
On Thu, Mar 31, 2016 at 3:12 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Wed, Mar 30, 2016 at 09:58:41PM +0530, haris iqbal wrote: >> On Wed, Mar 30, 2016 at 7:39 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: >> > On Thu, Mar 24, 2016 at 12:02:03AM +0530, Md Haris Iqbal wrote: >> >> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> >> >> --- >> >> linux-user/qemu.h | 4 ++-- >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h >> >> index 26b0ba2..3c3fd15 100644 >> >> --- a/linux-user/qemu.h >> >> +++ b/linux-user/qemu.h >> >> @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy >> >> #ifdef DEBUG_REMAP >> >> { >> >> void *addr; >> >> - addr = malloc(len); >> >> + addr = g_malloc(len); >> >> if (copy) >> >> memcpy(addr, g2h(guest_addr), len); >> >> else >> >> @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, >> >> return; >> >> if (len > 0) >> >> memcpy(g2h(guest_addr), host_ptr, len); >> >> - free(host_ptr); >> >> + g_free(host_ptr); >> >> #endif >> >> } >> > >> > If I understand correctly either lock_user() or lock_user_string() may >> > be followed by unlock_user(). If you change unlock_user() to g_free() >> > then you also need to change lock_user_string() to g_malloc() to avoid a >> > malloc()/g_free() mismatch. >> >> lock_user_string() does not use malloc itself, but calls lock_user() >> from itself. > > You are right. The reason I got confused is because there are 3 > versions of lock_user(): linux-user, bsd-user, and > include/exec/softmmu-semi.h. > > Please send equivalent patches for the other 2 versions as well. This > way they stay consistent. Will do. Actually as they different files, I was told to send patches separately. And I wanted to wait till this one is reviewed, so I don't repeat any mistake if there are in this one. > > Thanks, > Stefan
Just a reminder. any more changes needed for this patch? or can it be merged? On Thu, Mar 31, 2016 at 4:27 PM, haris iqbal <haris.phnx@gmail.com> wrote: > On Thu, Mar 31, 2016 at 3:12 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: >> On Wed, Mar 30, 2016 at 09:58:41PM +0530, haris iqbal wrote: >>> On Wed, Mar 30, 2016 at 7:39 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: >>> > On Thu, Mar 24, 2016 at 12:02:03AM +0530, Md Haris Iqbal wrote: >>> >> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> >>> >> --- >>> >> linux-user/qemu.h | 4 ++-- >>> >> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >> >>> >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h >>> >> index 26b0ba2..3c3fd15 100644 >>> >> --- a/linux-user/qemu.h >>> >> +++ b/linux-user/qemu.h >>> >> @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy >>> >> #ifdef DEBUG_REMAP >>> >> { >>> >> void *addr; >>> >> - addr = malloc(len); >>> >> + addr = g_malloc(len); >>> >> if (copy) >>> >> memcpy(addr, g2h(guest_addr), len); >>> >> else >>> >> @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, >>> >> return; >>> >> if (len > 0) >>> >> memcpy(g2h(guest_addr), host_ptr, len); >>> >> - free(host_ptr); >>> >> + g_free(host_ptr); >>> >> #endif >>> >> } >>> > >>> > If I understand correctly either lock_user() or lock_user_string() may >>> > be followed by unlock_user(). If you change unlock_user() to g_free() >>> > then you also need to change lock_user_string() to g_malloc() to avoid a >>> > malloc()/g_free() mismatch. >>> >>> lock_user_string() does not use malloc itself, but calls lock_user() >>> from itself. >> >> You are right. The reason I got confused is because there are 3 >> versions of lock_user(): linux-user, bsd-user, and >> include/exec/softmmu-semi.h. >> >> Please send equivalent patches for the other 2 versions as well. This >> way they stay consistent. > > Will do. Actually as they different files, I was told to send patches > separately. > > And I wanted to wait till this one is reviewed, so I don't repeat any > mistake if there are in this one. > >> >> Thanks, >> Stefan > > > > -- > > With regards, > > Md Haris Iqbal, > Placement Coordinator, MTech IT > NITK Surathkal, > Contact: +91 8861996962
On Thu, Mar 24, 2016 at 12:02:03AM +0530, Md Haris Iqbal wrote: > Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> > --- > linux-user/qemu.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
23.03.2016 21:32, Md Haris Iqbal wrote: > Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> > --- > linux-user/qemu.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 26b0ba2..3c3fd15 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy > #ifdef DEBUG_REMAP > { > void *addr; > - addr = malloc(len); > + addr = g_malloc(len); > if (copy) > memcpy(addr, g2h(guest_addr), len); > else > @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, > return; > if (len > 0) > memcpy(g2h(guest_addr), host_ptr, len); > - free(host_ptr); > + g_free(host_ptr); > #endif > } This is a very old patch, somehow missed by me at that time. Applied to -trivial now, thank you! /mjt
diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 26b0ba2..3c3fd15 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -381,7 +381,7 @@ static inline void *lock_user(int type, abi_ulong guest_addr, long len, int copy #ifdef DEBUG_REMAP { void *addr; - addr = malloc(len); + addr = g_malloc(len); if (copy) memcpy(addr, g2h(guest_addr), len); else @@ -407,7 +407,7 @@ static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, return; if (len > 0) memcpy(g2h(guest_addr), host_ptr, len); - free(host_ptr); + g_free(host_ptr); #endif }
Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com> --- linux-user/qemu.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)