From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dl9ES-0007Le-Ps for qemu-devel@nongnu.org; Fri, 25 Aug 2017 03:40:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dl9EP-0006k1-82 for qemu-devel@nongnu.org; Fri, 25 Aug 2017 03:40:44 -0400 Date: Fri, 25 Aug 2017 09:40:37 +0200 From: Igor Mammedov Message-ID: <20170825094037.4973225e@nial.brq.redhat.com> In-Reply-To: <20170825041644.GB2772@umbus.fritz.box> References: <1503562911-2776-1-git-send-email-imammedo@redhat.com> <1503562911-2776-6-git-send-email-imammedo@redhat.com> <20170825041644.GB2772@umbus.fritz.box> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.11 5/6] ppc: simplify cpu model lookup by PVR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Alexander Graf On Fri, 25 Aug 2017 14:16:44 +1000 David Gibson wrote: > On Thu, Aug 24, 2017 at 10:21:50AM +0200, Igor Mammedov wrote: > > Signed-off-by: Igor Mammedov > > --- > > target/ppc/translate_init.c | 27 +++++++++++---------------- > > 1 file changed, 11 insertions(+), 16 deletions(-) > > > > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c > > index f1a559d..ca9f1e3 100644 > > --- a/target/ppc/translate_init.c > > +++ b/target/ppc/translate_init.c > > @@ -34,6 +34,7 @@ > > #include "hw/ppc/ppc.h" > > #include "mmu-book3s-v3.h" > > #include "sysemu/qtest.h" > > +#include "qemu/cutils.h" > > > > //#define PPC_DUMP_CPU > > //#define PPC_DEBUG_SPR > > @@ -10203,22 +10204,16 @@ static ObjectClass *ppc_cpu_class_by_name(const char *name) > > char *cpu_model, *typename; > > ObjectClass *oc; > > const char *p; > > - int i, len; > > - > > - /* Check if the given name is a PVR */ > > - len = strlen(name); > > - if (len == 10 && name[0] == '0' && name[1] == 'x') { > > - p = name + 2; > > - goto check_pvr; > > - } else if (len == 8) { > > - p = name; > > - check_pvr: > > - for (i = 0; i < 8; i++) { > > - if (!qemu_isxdigit(*p++)) > > - break; > > - } > > - if (i == 8) { > > - return OBJECT_CLASS(ppc_cpu_class_by_pvr(strtoul(name, NULL, 16))); > > + unsigned long pvr; > > + > > + /* Lookup by PVR if cpu_model is valid 8 digit hex number > > + * (excl: 0x prefix if present) > > + */ > > + if (!qemu_strtoul(name, &p, 16, &pvr)) { > > + int len = p - name; > > + len = (len == 10) && (name[1] == 'x') ? len - 2 : len; > > + if (len == 8) { > > + return OBJECT_CLASS(ppc_cpu_class_by_pvr(pvr)); > > This doesn't look quite right. A hex string of a PVR followed by > other stuff isn't a valid option, so if p (endptr) doesn't point to > the end of the string, then we shouldn't be using this path yep, my mistake, I've lost somewhere *p == '\0' check when cleaning up the patch >(from the > looks of qemu_strtoul() we can simply omit the endptr parameter to get > that behaviour). I'm not sure what the messing around with len is > for, either. If it's a valid hex string we can try this, I don't see > that we need further checks. I've tried to keep current limitation here i.e. 8 hex symbols, but if any hex number is fine then doing qemu_strtoul(name, NULL, 16, &pvr) is even better, so would you prefer to drop 8 symbol check altogether? > > > } > > } > > >