On Tue, May 19, 2015 at 08:41:39AM +0800, Shannon Zhao wrote:
>
>
> On 2015/5/19 0:17, Michael S. Tsirkin wrote:
> > On Thu, May 14, 2015 at 05:19:33PM +0800, Shannon Zhao wrote:
> >> > From: Shannon Zhao <shannon.z...@linaro.org>
> >> >
> >> > Add ToUUID macro, this is useful for generating PCIe ACPI table.
> >> >
> >> > Signed-off-by: Shannon Zhao <zhaoshengl...@huawei.com>
> >> > Signed-off-by: Shannon Zhao <shannon.z...@linaro.org>
> >> > ---
> >> > hw/acpi/aml-build.c | 46
> >> > +++++++++++++++++++++++++++++++++++++++++++++
> >> > include/hw/acpi/aml-build.h | 1 +
> >> > 2 files changed, 47 insertions(+)
> >> >
> >> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> >> > index 6197896..0e4d35f 100644
> >> > --- a/hw/acpi/aml-build.c
> >> > +++ b/hw/acpi/aml-build.c
> >> > @@ -956,6 +956,52 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed
> >> > min_fixed,
> >> > addr_trans, len, flags);
> >> > }
> >> >
> >> > +static uint8_t Hex2Byte(const char *src)
> >> > +{
> >> > + int hi = Hex2Digit(*src++);
> >> > + int lo = Hex2Digit(*src);
> >
> > Just src[0] src[1] would be clearer.
> >
> >> > +
> >> > + g_assert(((hi >= 0) && (hi <= 15)) && ((lo >= 0) && (lo <= 15)));
> > Pls use simple assert everywhere, it does not matter
> > and rest of acpi uses plain assert.
> >
> >> > + return (hi << 4) | lo;
> >> > +}
> > Really should be lower-case. We need to fix Hex2Digit too,
> > but since Hex2Digit is broken, I'll let this one in
> > and we can fix both with a patch on top.
> >
> >> > +
> >> > +/*
> >> > + * ACPI 3.0: 17.5.124 ToUUID (Convert String to UUID Macro)
> >> > + * e.g. UUID: aabbccdd-eeff-gghh-iijj-kkllmmnnoopp
> >> > + * call aml_touuid("aabbccdd-eeff-gghh-iijj-kkllmmnnoopp");
> >> > + */
> >> > +Aml *aml_touuid(const char *uuid)
> >> > +{
> >> > + Aml *var = aml_bundle(0x11 /* BufferOp */, AML_BUFFER);
> >> > +
> >> > + /* format: aabbccdd-eeff-gghh-iijj-kkllmmnnoopp */
> > This documents the argument, so really belongs near the
> > function's head.
> >
> >> > + g_assert((strlen(uuid) == 36) && (uuid[8] == '-') && (uuid[13] ==
> >> > '-')
> >> > + && (uuid[18] == '-') && (uuid[23] == '-'));
> > Does something else validate the parameter?
> > It's not friendly to handle errors by assert.
> > Why doesn't that something else give us a pre-parse structure then?
> >
>
> a pre-parse structure?
Yes - have caller validate and parse the string, on failure report
it to user cleanly, on success give us a byte array
avoiding need to call Hex2Byte.
> >
> >> > +
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 6)); /* dd - at offset
> >> > 00 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 4)); /* cc - at offset
> >> > 01 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 2)); /* bb - at offset
> >> > 02 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 0)); /* aa - at offset
> >> > 03 */
> >> > +
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 11)); /* ff - at offset
> >> > 04 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 9)); /* ee - at offset
> >> > 05 */
> >> > +
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 16)); /* hh - at offset
> >> > 06 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 14)); /* gg - at offset
> >> > 07 */
> >> > +
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 19)); /* ii - at offset
> >> > 08 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 21)); /* jj - at offset
> >> > 09 */
> >> > +
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 24)); /* kk - at offset
> >> > 10 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 26)); /* ll - at offset
> >> > 11 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 28)); /* mm - at offset
> >> > 12 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 30)); /* nn - at offset
> >> > 13 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 32)); /* oo - at offset
> >> > 14 */
> >> > + build_append_byte(var->buf, Hex2Byte(uuid + 34)); /* pp - at offset
> >> > 15 */
> > Can we use strtoull?
> >
>
> Igor and I have a discussion on this. I used strtol before and we got an
> agreement on current approach.
>
> --
> Shannon
