All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
@ 2018-03-11 20:12 Nia Alarie
  2018-03-12  9:01 ` Greg Kurz
  2018-03-12 12:12 ` Eric Blake
  0 siblings, 2 replies; 10+ messages in thread
From: Nia Alarie @ 2018-03-11 20:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: groug, stefanha, jim, joel, Nia Alarie

Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
---
 hw/9pfs/9p.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 48fa48e720..64f3bb976c 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -15,6 +15,7 @@
 #include <glib/gprintf.h>
 #include "hw/virtio/virtio.h"
 #include "qapi/error.h"
+#include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/iov.h"
 #include "qemu/sockets.h"
@@ -2213,8 +2214,15 @@ static void coroutine_fn v9fs_create(void *opaque)
         }
         v9fs_path_copy(&fidp->path, &path);
     } else if (perm & P9_STAT_MODE_LINK) {
-        int32_t ofid = atoi(extension.data);
-        V9fsFidState *ofidp = get_fid(pdu, ofid);
+        long ofid;
+        V9fsFidState *ofidp;
+
+        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
+            ofid > INT32_MAX || ofid < INT32_MIN) {
+            err = -EINVAL;
+            goto out;
+        }
+        ofidp = get_fid(pdu, (int32_t)ofid);
         if (ofidp == NULL) {
             err = -EINVAL;
             goto out;
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-11 20:12 [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking Nia Alarie
@ 2018-03-12  9:01 ` Greg Kurz
  2018-03-12 13:16   ` Greg Kurz
  2018-03-12 12:12 ` Eric Blake
  1 sibling, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2018-03-12  9:01 UTC (permalink / raw)
  To: Nia Alarie; +Cc: qemu-devel, stefanha, jim, joel

On Sun, 11 Mar 2018 20:12:39 +0000
Nia Alarie <nia.alarie@gmail.com> wrote:

> Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> ---

Applied, thanks.

>  hw/9pfs/9p.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 48fa48e720..64f3bb976c 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -15,6 +15,7 @@
>  #include <glib/gprintf.h>
>  #include "hw/virtio/virtio.h"
>  #include "qapi/error.h"
> +#include "qemu/cutils.h"
>  #include "qemu/error-report.h"
>  #include "qemu/iov.h"
>  #include "qemu/sockets.h"
> @@ -2213,8 +2214,15 @@ static void coroutine_fn v9fs_create(void *opaque)
>          }
>          v9fs_path_copy(&fidp->path, &path);
>      } else if (perm & P9_STAT_MODE_LINK) {
> -        int32_t ofid = atoi(extension.data);
> -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> +        long ofid;
> +        V9fsFidState *ofidp;
> +
> +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> +            ofid > INT32_MAX || ofid < INT32_MIN) {
> +            err = -EINVAL;
> +            goto out;
> +        }
> +        ofidp = get_fid(pdu, (int32_t)ofid);
>          if (ofidp == NULL) {
>              err = -EINVAL;
>              goto out;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-11 20:12 [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking Nia Alarie
  2018-03-12  9:01 ` Greg Kurz
@ 2018-03-12 12:12 ` Eric Blake
  2018-03-12 13:02   ` Greg Kurz
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Blake @ 2018-03-12 12:12 UTC (permalink / raw)
  To: Nia Alarie, qemu-devel; +Cc: stefanha, jim, groug, joel, Daniel P. Berrange

On 03/11/2018 03:12 PM, Nia Alarie wrote:
> Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> ---
>   hw/9pfs/9p.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 

>       } else if (perm & P9_STAT_MODE_LINK) {
> -        int32_t ofid = atoi(extension.data);
> -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> +        long ofid;
> +        V9fsFidState *ofidp;
> +
> +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> +            ofid > INT32_MAX || ofid < INT32_MIN) {

Dan has a pending patch that will add qemu_strtoi, which might be a 
nicer fit for this situation:

https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html

int32_t is not necessarily int, but all platforms that compile qemu have 
'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int 
ofid' and use Dan's function than it is to parse to long and then do 
bounds checking.  Except that Dan still needs to post an updated version 
of his patch...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-12 12:12 ` Eric Blake
@ 2018-03-12 13:02   ` Greg Kurz
  2018-03-12 13:08     ` Daniel P. Berrangé
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2018-03-12 13:02 UTC (permalink / raw)
  To: Eric Blake
  Cc: Nia Alarie, qemu-devel, stefanha, jim, joel, Daniel P. Berrange

On Mon, 12 Mar 2018 07:12:52 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 03/11/2018 03:12 PM, Nia Alarie wrote:
> > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> > ---
> >   hw/9pfs/9p.c | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >   
> 
> >       } else if (perm & P9_STAT_MODE_LINK) {
> > -        int32_t ofid = atoi(extension.data);
> > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> > +        long ofid;
> > +        V9fsFidState *ofidp;
> > +
> > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> > +            ofid > INT32_MAX || ofid < INT32_MIN) {  
> 
> Dan has a pending patch that will add qemu_strtoi, which might be a 
> nicer fit for this situation:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html
> 
> int32_t is not necessarily int, but all platforms that compile qemu have 
> 'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int 
> ofid' and use Dan's function than it is to parse to long and then do 
> bounds checking.  Except that Dan still needs to post an updated version 
> of his patch...
> 

I wasn't aware of Dan's patch but I agree it would result in a nicer
change for 9p. This being said, tomorrow is soft freeze... is there
a chance Dan's patch reaches master anytime soon ?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-12 13:02   ` Greg Kurz
@ 2018-03-12 13:08     ` Daniel P. Berrangé
  2018-03-12 13:21       ` Greg Kurz
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrangé @ 2018-03-12 13:08 UTC (permalink / raw)
  To: Greg Kurz; +Cc: Eric Blake, Nia Alarie, qemu-devel, stefanha, jim, joel

On Mon, Mar 12, 2018 at 02:02:29PM +0100, Greg Kurz wrote:
> On Mon, 12 Mar 2018 07:12:52 -0500
> Eric Blake <eblake@redhat.com> wrote:
> 
> > On 03/11/2018 03:12 PM, Nia Alarie wrote:
> > > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> > > ---
> > >   hw/9pfs/9p.c | 12 ++++++++++--
> > >   1 file changed, 10 insertions(+), 2 deletions(-)
> > >   
> > 
> > >       } else if (perm & P9_STAT_MODE_LINK) {
> > > -        int32_t ofid = atoi(extension.data);
> > > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> > > +        long ofid;
> > > +        V9fsFidState *ofidp;
> > > +
> > > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> > > +            ofid > INT32_MAX || ofid < INT32_MIN) {  
> > 
> > Dan has a pending patch that will add qemu_strtoi, which might be a 
> > nicer fit for this situation:
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html
> > 
> > int32_t is not necessarily int, but all platforms that compile qemu have 
> > 'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int 
> > ofid' and use Dan's function than it is to parse to long and then do 
> > bounds checking.  Except that Dan still needs to post an updated version 
> > of his patch...
> > 
> 
> I wasn't aware of Dan's patch but I agree it would result in a nicer
> change for 9p. This being said, tomorrow is soft freeze... is there
> a chance Dan's patch reaches master anytime soon ?

I just sent an update of my series. If it gets acked by Eric, I'd be able
to send a pull request today.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-12  9:01 ` Greg Kurz
@ 2018-03-12 13:16   ` Greg Kurz
  0 siblings, 0 replies; 10+ messages in thread
From: Greg Kurz @ 2018-03-12 13:16 UTC (permalink / raw)
  To: Nia Alarie; +Cc: stefanha, jim, qemu-devel, joel

On Mon, 12 Mar 2018 10:01:46 +0100
Greg Kurz <groug@kaod.org> wrote:

> On Sun, 11 Mar 2018 20:12:39 +0000
> Nia Alarie <nia.alarie@gmail.com> wrote:
> 
> > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> > ---  
> 
> Applied, thanks.
> 

Following Eric's suggestion in another mail, let's give a chance for
the new qemu_strto*() helpers to reach master.

Also, FIDs are unsigned 32-bit integers, so we should use a qemu_strtou*()
variant.

> >  hw/9pfs/9p.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 48fa48e720..64f3bb976c 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -15,6 +15,7 @@
> >  #include <glib/gprintf.h>
> >  #include "hw/virtio/virtio.h"
> >  #include "qapi/error.h"
> > +#include "qemu/cutils.h"
> >  #include "qemu/error-report.h"
> >  #include "qemu/iov.h"
> >  #include "qemu/sockets.h"
> > @@ -2213,8 +2214,15 @@ static void coroutine_fn v9fs_create(void *opaque)
> >          }
> >          v9fs_path_copy(&fidp->path, &path);
> >      } else if (perm & P9_STAT_MODE_LINK) {
> > -        int32_t ofid = atoi(extension.data);
> > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> > +        long ofid;
> > +        V9fsFidState *ofidp;
> > +
> > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> > +            ofid > INT32_MAX || ofid < INT32_MIN) {
> > +            err = -EINVAL;
> > +            goto out;
> > +        }
> > +        ofidp = get_fid(pdu, (int32_t)ofid);
> >          if (ofidp == NULL) {
> >              err = -EINVAL;
> >              goto out;  
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-12 13:08     ` Daniel P. Berrangé
@ 2018-03-12 13:21       ` Greg Kurz
  2018-03-13 15:25         ` nee
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kurz @ 2018-03-12 13:21 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Eric Blake, Nia Alarie, qemu-devel, stefanha, jim, joel

On Mon, 12 Mar 2018 13:08:29 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Mar 12, 2018 at 02:02:29PM +0100, Greg Kurz wrote:
> > On Mon, 12 Mar 2018 07:12:52 -0500
> > Eric Blake <eblake@redhat.com> wrote:
> >   
> > > On 03/11/2018 03:12 PM, Nia Alarie wrote:  
> > > > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> > > > ---
> > > >   hw/9pfs/9p.c | 12 ++++++++++--
> > > >   1 file changed, 10 insertions(+), 2 deletions(-)
> > > >     
> > >   
> > > >       } else if (perm & P9_STAT_MODE_LINK) {
> > > > -        int32_t ofid = atoi(extension.data);
> > > > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> > > > +        long ofid;
> > > > +        V9fsFidState *ofidp;
> > > > +
> > > > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> > > > +            ofid > INT32_MAX || ofid < INT32_MIN) {    
> > > 
> > > Dan has a pending patch that will add qemu_strtoi, which might be a 
> > > nicer fit for this situation:
> > > 
> > > https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html
> > > 
> > > int32_t is not necessarily int, but all platforms that compile qemu have 
> > > 'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int 
> > > ofid' and use Dan's function than it is to parse to long and then do 
> > > bounds checking.  Except that Dan still needs to post an updated version 
> > > of his patch...
> > >   
> > 
> > I wasn't aware of Dan's patch but I agree it would result in a nicer
> > change for 9p. This being said, tomorrow is soft freeze... is there
> > a chance Dan's patch reaches master anytime soon ?  
> 
> I just sent an update of my series. If it gets acked by Eric, I'd be able
> to send a pull request today.
> 
> Regards,
> Daniel

Great !

Nia,

Could you please respin your patch on top of Daniel's series, using
qemu_strtoui() ?

Thanks,

--
Greg

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-12 13:21       ` Greg Kurz
@ 2018-03-13 15:25         ` nee
  2018-03-13 15:52           ` Stefan Hajnoczi
  0 siblings, 1 reply; 10+ messages in thread
From: nee @ 2018-03-13 15:25 UTC (permalink / raw)
  To: Greg Kurz
  Cc: Daniel P. Berrangé,
	Eric Blake, qemu-devel, Stefan Hajnoczi, Jim Mussared,
	Joel Stanley

On Mon, Mar 12, 2018 at 1:21 PM, Greg Kurz <groug@kaod.org> wrote:
> On Mon, 12 Mar 2018 13:08:29 +0000
> Daniel P. Berrangé <berrange@redhat.com> wrote:
>
>> On Mon, Mar 12, 2018 at 02:02:29PM +0100, Greg Kurz wrote:
>> > On Mon, 12 Mar 2018 07:12:52 -0500
>> > Eric Blake <eblake@redhat.com> wrote:
>> >
>> > > On 03/11/2018 03:12 PM, Nia Alarie wrote:
>> > > > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
>> > > > ---
>> > > >   hw/9pfs/9p.c | 12 ++++++++++--
>> > > >   1 file changed, 10 insertions(+), 2 deletions(-)
>> > > >
>> > >
>> > > >       } else if (perm & P9_STAT_MODE_LINK) {
>> > > > -        int32_t ofid = atoi(extension.data);
>> > > > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
>> > > > +        long ofid;
>> > > > +        V9fsFidState *ofidp;
>> > > > +
>> > > > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
>> > > > +            ofid > INT32_MAX || ofid < INT32_MIN) {
>> > >
>> > > Dan has a pending patch that will add qemu_strtoi, which might be a
>> > > nicer fit for this situation:
>> > >
>> > > https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html
>> > >
>> > > int32_t is not necessarily int, but all platforms that compile qemu have
>> > > 'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int
>> > > ofid' and use Dan's function than it is to parse to long and then do
>> > > bounds checking.  Except that Dan still needs to post an updated version
>> > > of his patch...
>> > >
>> >
>> > I wasn't aware of Dan's patch but I agree it would result in a nicer
>> > change for 9p. This being said, tomorrow is soft freeze... is there
>> > a chance Dan's patch reaches master anytime soon ?
>>
>> I just sent an update of my series. If it gets acked by Eric, I'd be able
>> to send a pull request today.
>>
>> Regards,
>> Daniel
>
> Great !
>
> Nia,
>
> Could you please respin your patch on top of Daniel's series, using
> qemu_strtoui() ?
>
> Thanks,
>
> --
> Greg

I'm a bit unclear on how this works so I thought I'd ask here before I
send any more patches - this is CCed to my mentors as well. It's
"tomorrow" now, but I don't understand how soft freezes work or the
process beyond "I send this patch to qemu-devel and people say if I
should re-send it with changes, or that it's been accepted" :)

There are several more conversions from ato* to qemu_strto* I'd like
to submit from now on. Should I send them as using qemu_strtol now, to
get them "out there" before my own personal deadlines, and send
further patches to convert them to qemu_strtoi later, or should I
submit patches that use qemu_strtoi? I'm not sure of the current
status of Daniel's patches.

Thank you.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-13 15:25         ` nee
@ 2018-03-13 15:52           ` Stefan Hajnoczi
  2018-03-13 16:28             ` Greg Kurz
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Hajnoczi @ 2018-03-13 15:52 UTC (permalink / raw)
  To: nee
  Cc: Greg Kurz, Daniel P. Berrangé,
	Eric Blake, qemu-devel, Jim Mussared, Joel Stanley

On Tue, Mar 13, 2018 at 3:25 PM, nee <nia.alarie@gmail.com> wrote:
> On Mon, Mar 12, 2018 at 1:21 PM, Greg Kurz <groug@kaod.org> wrote:
>> On Mon, 12 Mar 2018 13:08:29 +0000
>> Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>>> On Mon, Mar 12, 2018 at 02:02:29PM +0100, Greg Kurz wrote:
>>> > On Mon, 12 Mar 2018 07:12:52 -0500
>>> > Eric Blake <eblake@redhat.com> wrote:
>>> >
>>> > > On 03/11/2018 03:12 PM, Nia Alarie wrote:
>>> > > > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
>>> > > > ---
>>> > > >   hw/9pfs/9p.c | 12 ++++++++++--
>>> > > >   1 file changed, 10 insertions(+), 2 deletions(-)
>>> > > >
>>> > >
>>> > > >       } else if (perm & P9_STAT_MODE_LINK) {
>>> > > > -        int32_t ofid = atoi(extension.data);
>>> > > > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
>>> > > > +        long ofid;
>>> > > > +        V9fsFidState *ofidp;
>>> > > > +
>>> > > > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
>>> > > > +            ofid > INT32_MAX || ofid < INT32_MIN) {
>>> > >
>>> > > Dan has a pending patch that will add qemu_strtoi, which might be a
>>> > > nicer fit for this situation:
>>> > >
>>> > > https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html
>>> > >
>>> > > int32_t is not necessarily int, but all platforms that compile qemu have
>>> > > 'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int
>>> > > ofid' and use Dan's function than it is to parse to long and then do
>>> > > bounds checking.  Except that Dan still needs to post an updated version
>>> > > of his patch...
>>> > >
>>> >
>>> > I wasn't aware of Dan's patch but I agree it would result in a nicer
>>> > change for 9p. This being said, tomorrow is soft freeze... is there
>>> > a chance Dan's patch reaches master anytime soon ?
>>>
>>> I just sent an update of my series. If it gets acked by Eric, I'd be able
>>> to send a pull request today.
>>>
>>> Regards,
>>> Daniel
>>
>> Great !
>>
>> Nia,
>>
>> Could you please respin your patch on top of Daniel's series, using
>> qemu_strtoui() ?
>>
>> Thanks,
>>
>> --
>> Greg
>
> I'm a bit unclear on how this works so I thought I'd ask here before I
> send any more patches - this is CCed to my mentors as well. It's
> "tomorrow" now, but I don't understand how soft freezes work or the
> process beyond "I send this patch to qemu-devel and people say if I
> should re-send it with changes, or that it's been accepted" :)

Don't worry about the soft freeze for now.  The soft freeze is the
deadline for new features.  People are trying to get their new
features into the upcoming QEMU 2.12 release.

Either Greg will still merge your patch after soft freeze since it can
be considered a bug fix (fixes are allowed during freeze).  Or Greg
might keep your patch queued until the QEMU 2.12 release has been made
(around April 17th 2018).

The QEMU release schedule is here (but don't worry about it):
https://wiki.qemu.org/Planning/2.12

I suggest taking the following action.  Wait until Daniel Berrange's
pull request is merged (it will probably happen today):
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03662.html

Then rebase your patch onto the latest qemu.git/master so you can take
advantage of qemu_strtoi().  You can use "git rebase -i origin/master"
to do this.

Recompile/test your patch and then send it with "[PATCH v2] 9p:
Convert use of atoi to qemu_strtoi to allow error checking" as the
email subject.

> There are several more conversions from ato* to qemu_strto* I'd like
> to submit from now on. Should I send them as using qemu_strtol now, to
> get them "out there" before my own personal deadlines, and send
> further patches to convert them to qemu_strtoi later, or should I
> submit patches that use qemu_strtoi? I'm not sure of the current
> status of Daniel's patches.

Daniel's patches will be in qemu.git/master (probably) later today.  I
suggest waiting for them and using qemu_strtoi() where using an int
type is appropriate.

If you have questions along the way, feel free to ask on the QEMU IRC
channel: #qemu on irc.oftc.net.

Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking
  2018-03-13 15:52           ` Stefan Hajnoczi
@ 2018-03-13 16:28             ` Greg Kurz
  0 siblings, 0 replies; 10+ messages in thread
From: Greg Kurz @ 2018-03-13 16:28 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: nee, Daniel P. Berrangé,
	Eric Blake, qemu-devel, Jim Mussared, Joel Stanley

On Tue, 13 Mar 2018 15:52:41 +0000
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, Mar 13, 2018 at 3:25 PM, nee <nia.alarie@gmail.com> wrote:
> > On Mon, Mar 12, 2018 at 1:21 PM, Greg Kurz <groug@kaod.org> wrote:  
> >> On Mon, 12 Mar 2018 13:08:29 +0000
> >> Daniel P. Berrangé <berrange@redhat.com> wrote:
> >>  
> >>> On Mon, Mar 12, 2018 at 02:02:29PM +0100, Greg Kurz wrote:  
> >>> > On Mon, 12 Mar 2018 07:12:52 -0500
> >>> > Eric Blake <eblake@redhat.com> wrote:
> >>> >  
> >>> > > On 03/11/2018 03:12 PM, Nia Alarie wrote:  
> >>> > > > Signed-off-by: Nia Alarie <nia.alarie@gmail.com>
> >>> > > > ---
> >>> > > >   hw/9pfs/9p.c | 12 ++++++++++--
> >>> > > >   1 file changed, 10 insertions(+), 2 deletions(-)
> >>> > > >  
> >>> > >  
> >>> > > >       } else if (perm & P9_STAT_MODE_LINK) {
> >>> > > > -        int32_t ofid = atoi(extension.data);
> >>> > > > -        V9fsFidState *ofidp = get_fid(pdu, ofid);
> >>> > > > +        long ofid;
> >>> > > > +        V9fsFidState *ofidp;
> >>> > > > +
> >>> > > > +        if (qemu_strtol(extension.data, NULL, 10, &ofid) ||
> >>> > > > +            ofid > INT32_MAX || ofid < INT32_MIN) {  
> >>> > >
> >>> > > Dan has a pending patch that will add qemu_strtoi, which might be a
> >>> > > nicer fit for this situation:
> >>> > >
> >>> > > https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00952.html
> >>> > >
> >>> > > int32_t is not necessarily int, but all platforms that compile qemu have
> >>> > > 'int32_t' and 'int' both at 32 bits, so it's simpler to change to 'int
> >>> > > ofid' and use Dan's function than it is to parse to long and then do
> >>> > > bounds checking.  Except that Dan still needs to post an updated version
> >>> > > of his patch...
> >>> > >  
> >>> >
> >>> > I wasn't aware of Dan's patch but I agree it would result in a nicer
> >>> > change for 9p. This being said, tomorrow is soft freeze... is there
> >>> > a chance Dan's patch reaches master anytime soon ?  
> >>>
> >>> I just sent an update of my series. If it gets acked by Eric, I'd be able
> >>> to send a pull request today.
> >>>
> >>> Regards,
> >>> Daniel  
> >>
> >> Great !
> >>
> >> Nia,
> >>
> >> Could you please respin your patch on top of Daniel's series, using
> >> qemu_strtoui() ?
> >>
> >> Thanks,
> >>
> >> --
> >> Greg  
> >
> > I'm a bit unclear on how this works so I thought I'd ask here before I
> > send any more patches - this is CCed to my mentors as well. It's
> > "tomorrow" now, but I don't understand how soft freezes work or the
> > process beyond "I send this patch to qemu-devel and people say if I
> > should re-send it with changes, or that it's been accepted" :)  
> 
> Don't worry about the soft freeze for now.  The soft freeze is the
> deadline for new features.  People are trying to get their new
> features into the upcoming QEMU 2.12 release.
> 
> Either Greg will still merge your patch after soft freeze since it can
> be considered a bug fix (fixes are allowed during freeze).  Or Greg
> might keep your patch queued until the QEMU 2.12 release has been made
> (around April 17th 2018).
> 

Nia (or Nee?) 's patch is a bug fix. I'll happily merge it when it's
ready.

> The QEMU release schedule is here (but don't worry about it):
> https://wiki.qemu.org/Planning/2.12
> 
> I suggest taking the following action.  Wait until Daniel Berrange's
> pull request is merged (it will probably happen today):
> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03662.html
> 
> Then rebase your patch onto the latest qemu.git/master so you can take
> advantage of qemu_strtoi().  You can use "git rebase -i origin/master"
> to do this.
> 
> Recompile/test your patch and then send it with "[PATCH v2] 9p:
> Convert use of atoi to qemu_strtoi to allow error checking" as the
> email subject.
> 

Nia had already sent a v2, which got comments, so it'll be v3.

> > There are several more conversions from ato* to qemu_strto* I'd like
> > to submit from now on. Should I send them as using qemu_strtol now, to
> > get them "out there" before my own personal deadlines, and send
> > further patches to convert them to qemu_strtoi later, or should I
> > submit patches that use qemu_strtoi? I'm not sure of the current
> > status of Daniel's patches.  
> 
> Daniel's patches will be in qemu.git/master (probably) later today.  I
> suggest waiting for them and using qemu_strtoi() where using an int
> type is appropriate.
> 
> If you have questions along the way, feel free to ask on the QEMU IRC
> channel: #qemu on irc.oftc.net.
> 
> Stefan

Agreed with all of the above. Thanks Stefan for providing this comprehensive
explanation to Nia.

Cheers,

--
Greg

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-03-13 16:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-11 20:12 [Qemu-devel] [PATCH] 9p: Convert use of atoi to qemu_strtol to allow error checking Nia Alarie
2018-03-12  9:01 ` Greg Kurz
2018-03-12 13:16   ` Greg Kurz
2018-03-12 12:12 ` Eric Blake
2018-03-12 13:02   ` Greg Kurz
2018-03-12 13:08     ` Daniel P. Berrangé
2018-03-12 13:21       ` Greg Kurz
2018-03-13 15:25         ` nee
2018-03-13 15:52           ` Stefan Hajnoczi
2018-03-13 16:28             ` Greg Kurz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.