| Message ID | 1372943363-24081-8-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04.07.2013, at 15:09, Markus Armbruster wrote: > Make it QEMU_OPT_NUMBER, so it gets parsed by generic code, which > actually bothers to check for errors, rather than its user, which > doesn't. > > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > device_tree.c | 7 ++----- > vl.c | 2 +- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/device_tree.c b/device_tree.c > index 0e7fe2d..10cf3d0 100644 > --- a/device_tree.c > +++ b/device_tree.c > @@ -240,11 +240,8 @@ uint32_t qemu_devtree_alloc_phandle(void *fdt) > * which phandle id to start allocting phandles. > */ > if (!phandle) { > - const char *phandle_start = qemu_opt_get(qemu_get_machine_opts(), > - "phandle_start"); > - if (phandle_start) { > - phandle = strtoul(phandle_start, NULL, 0); > - } > + phandle = qemu_opt_get_number(qemu_get_machine_opts(), > + "phandle_start", 0); Zero is a valid phandle to start from. It shouldn't mean "default". Alex > } > > if (!phandle) { > diff --git a/vl.c b/vl.c > index fb69f22..bea1a10 100644 > --- a/vl.c > +++ b/vl.c > @@ -409,7 +409,7 @@ static QemuOptsList qemu_machine_opts = { > .help = "Dump current dtb to a file and quit", > }, { > .name = "phandle_start", > - .type = QEMU_OPT_STRING, > + .type = QEMU_OPT_NUMBER, > .help = "The first phandle ID we may generate dynamically", > }, { > .name = "dt_compatible", > -- > 1.7.11.7 >
Alexander Graf <agraf@suse.de> writes: > On 04.07.2013, at 15:09, Markus Armbruster wrote: > >> Make it QEMU_OPT_NUMBER, so it gets parsed by generic code, which >> actually bothers to check for errors, rather than its user, which >> doesn't. >> >> Cc: Alexander Graf <agraf@suse.de> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> device_tree.c | 7 ++----- >> vl.c | 2 +- >> 2 files changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/device_tree.c b/device_tree.c >> index 0e7fe2d..10cf3d0 100644 >> --- a/device_tree.c >> +++ b/device_tree.c >> @@ -240,11 +240,8 @@ uint32_t qemu_devtree_alloc_phandle(void *fdt) uint32_t qemu_devtree_alloc_phandle(void *fdt) { static int phandle = 0x0; /* >> * which phandle id to start allocting phandles. >> */ >> if (!phandle) { >> - const char *phandle_start = qemu_opt_get(qemu_get_machine_opts(), >> - "phandle_start"); >> - if (phandle_start) { >> - phandle = strtoul(phandle_start, NULL, 0); >> - } >> + phandle = qemu_opt_get_number(qemu_get_machine_opts(), >> + "phandle_start", 0); > > Zero is a valid phandle to start from. It shouldn't mean "default". We get here only when phandle is zero (which it initially is). If opts don't contain a value for "phandle_start", we set phandle to zero, i.e. do nothing. Exactly the same as before. If that's wrong, it should be fixed in a separate patch.
On 04.07.2013, at 17:01, Markus Armbruster wrote: > Alexander Graf <agraf@suse.de> writes: > >> On 04.07.2013, at 15:09, Markus Armbruster wrote: >> >>> Make it QEMU_OPT_NUMBER, so it gets parsed by generic code, which >>> actually bothers to check for errors, rather than its user, which >>> doesn't. >>> >>> Cc: Alexander Graf <agraf@suse.de> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> device_tree.c | 7 ++----- >>> vl.c | 2 +- >>> 2 files changed, 3 insertions(+), 6 deletions(-) >>> >>> diff --git a/device_tree.c b/device_tree.c >>> index 0e7fe2d..10cf3d0 100644 >>> --- a/device_tree.c >>> +++ b/device_tree.c >>> @@ -240,11 +240,8 @@ uint32_t qemu_devtree_alloc_phandle(void *fdt) > uint32_t qemu_devtree_alloc_phandle(void *fdt) > { > static int phandle = 0x0; > > /* >>> * which phandle id to start allocting phandles. >>> */ >>> if (!phandle) { >>> - const char *phandle_start = qemu_opt_get(qemu_get_machine_opts(), >>> - "phandle_start"); >>> - if (phandle_start) { >>> - phandle = strtoul(phandle_start, NULL, 0); >>> - } >>> + phandle = qemu_opt_get_number(qemu_get_machine_opts(), >>> + "phandle_start", 0); >> >> Zero is a valid phandle to start from. It shouldn't mean "default". > > We get here only when phandle is zero (which it initially is). > > If opts don't contain a value for "phandle_start", we set phandle to > zero, i.e. do nothing. Exactly the same as before. True. Sorry for the fuss. Acked-by: Alexander Graf <agraf@suse.de> Alex > > If that's wrong, it should be fixed in a separate patch.
diff --git a/device_tree.c b/device_tree.c index 0e7fe2d..10cf3d0 100644 --- a/device_tree.c +++ b/device_tree.c @@ -240,11 +240,8 @@ uint32_t qemu_devtree_alloc_phandle(void *fdt) * which phandle id to start allocting phandles. */ if (!phandle) { - const char *phandle_start = qemu_opt_get(qemu_get_machine_opts(), - "phandle_start"); - if (phandle_start) { - phandle = strtoul(phandle_start, NULL, 0); - } + phandle = qemu_opt_get_number(qemu_get_machine_opts(), + "phandle_start", 0); } if (!phandle) { diff --git a/vl.c b/vl.c index fb69f22..bea1a10 100644 --- a/vl.c +++ b/vl.c @@ -409,7 +409,7 @@ static QemuOptsList qemu_machine_opts = { .help = "Dump current dtb to a file and quit", }, { .name = "phandle_start", - .type = QEMU_OPT_STRING, + .type = QEMU_OPT_NUMBER, .help = "The first phandle ID we may generate dynamically", }, { .name = "dt_compatible",
Make it QEMU_OPT_NUMBER, so it gets parsed by generic code, which actually bothers to check for errors, rather than its user, which doesn't. Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Markus Armbruster <armbru@redhat.com> --- device_tree.c | 7 ++----- vl.c | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-)