Re: [Qemu-devel] [PATCH v5 13/20] hw/acpi/aml-build: Add ToUUID macro

[Shannon Zhao <shannon.zhao@linaro.org>]

On 2015/4/28 17:48, Shannon Zhao wrote:
> On 2015/4/28 17:35, Igor Mammedov wrote:
>> On Tue, 28 Apr 2015 16:52:19 +0800
>> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>
>>> On 2015/4/28 16:15, Igor Mammedov wrote:
>>>>>> btw:
>>>>>>>> whole thing might be simpler if an intermediate conversion is avoided,
>>>>>>>> just pack buffer as in spec byte by byte:
>>>>>>>>
>>>>>>>> /* format: aabbccdd-eeff-gghh-iijj-kkllmmnnoopp */
>>>>>>>> assert(strlen(uuid) == ...);
>>>>>>>> build_append_byte(var->buf, HEX2BYTE(uuid[3]); /* dd */
>>>>>>
>>>>>> use build_append_byte(var->buf, HEX2BYTE(uuid + 7); ?
>>>>>>
>>>>>>>> build_append_byte(var->buf, HEX2BYTE(uuid[2]); /* cc */
>>>>>>
>>>>>> use build_append_byte(var->buf, HEX2BYTE(uuid + 5); ?
>>>> if you mean hyphens /-/ then they are not encoded,
>>>> but you surely can add checks for them to make sure that
>>>> UUID format is as expected.
>>>>
>>>
>>> I mean uuid[3] points to b not dd. Maybe use following way:
>>>
>>> static uint8_t Hex2Byte(char *src)
>> or even better:
>> Hex2Byte(char *src, byte_idx)
>>    and do pointer arithmetic inside
>>
>> [...]
>>> build_append_byte(var->buf, Hex2Byte(uuid + (3 * 2))); /* dd */
>> build_append_byte(var->buf, Hex2Byte(uuid, 3)); /* dd - at offset 00 */
>> build_append_byte(var->buf, Hex2Byte(uuid, 2)); /* cc - at offset 01 */
>> ...
>>
> Yes, it's better to first four bytes. But there are hyphens /-/, for
> offset 04, 05 and etc it doesn't work. We can't use following expression:
>
> build_append_byte(var->buf, Hex2Byte(uuid, 5)); /* ff - at offset 04 */
> build_append_byte(var->buf, Hex2Byte(uuid, 4)); /* ee - at offset 05 */
> ...
>
>

So about the implementation of this macro, I think I'd like to use 
following approach. This lets Hex2Byte do what it should only do and 
still has a clear look of UUID. What do you think about?

static uint8_t Hex2Byte(char *src)
{
     int hi = Hex2Digit(*src++);
     int lo = Hex2Digit(*src);

     if ((hi < 0) || (lo < 0))
         return -1;

     return (hi << 4) | lo;
}

g_assert((strlen(uuid) == 36) && (uuid[8] == '-') && (uuid[13] == '-')
           && (uuid[18] == '-') && (uuid[23] == '-'));

build_append_byte(var->buf, Hex2Byte(uuid + (3 * 2))); /* dd */
build_append_byte(var->buf, Hex2Byte(uuid + (2 * 2))); /* cc */
build_append_byte(var->buf, Hex2Byte(uuid + (1 * 2))); /* bb */
build_append_byte(var->buf, Hex2Byte(uuid + (0 * 2))); /* aa */

build_append_byte(var->buf, Hex2Byte(uuid + (5 * 2 + 1))); /* ff */
build_append_byte(var->buf, Hex2Byte(uuid + (4 * 2 + 1))); /* ee */

build_append_byte(var->buf, Hex2Byte(uuid + (7 * 2 + 2))); /* hh */
build_append_byte(var->buf, Hex2Byte(uuid + (6 * 2 + 2))); /* gg */

build_append_byte(var->buf, Hex2Byte(uuid + (8 * 2 + 3))); /* ii */
build_append_byte(var->buf, Hex2Byte(uuid + (9 * 2 + 3))); /* jj */

build_append_byte(var->buf, Hex2Byte(uuid + (10 * 2 + 4))); /* kk */
build_append_byte(var->buf, Hex2Byte(uuid + (11 * 2 + 4))); /* ll */
build_append_byte(var->buf, Hex2Byte(uuid + (12 * 2 + 4))); /* mm */
build_append_byte(var->buf, Hex2Byte(uuid + (13 * 2 + 4))); /* nn */
build_append_byte(var->buf, Hex2Byte(uuid + (14 * 2 + 4))); /* oo */
build_append_byte(var->buf, Hex2Byte(uuid + (15 * 2 + 4))); /* pp */