[PATCH v3 2/3] dtc: Support character literals in cell lists

[Anton Staaf]

On Thu, Sep 8, 2011 at 6:12 PM, David Gibson
<david at gibson.dropbear.id.au> wrote:
> On Thu, Sep 08, 2011 at 02:39:16PM -0700, Anton Staaf wrote:
>> With this patch the following property assignment:
>>
>>     property = <0x12345678 'a' '\r' 100>;
>>
>> is equivalent to:
>>
>>     property = <0x12345678 0x00000061 0x0000000D 0x00000064>
>
> One tiny nit..
>
> [snip]
>> diff --git a/dtc-lexer.l b/dtc-lexer.l
>> index e866ea5..94151da 100644
>> --- a/dtc-lexer.l
>> +++ b/dtc-lexer.l
>> @@ -29,6 +29,7 @@ PROPNODECHAR        [a-zA-Z0-9,._+*#?@-]
>>  PATHCHAR     ({PROPNODECHAR}|[/])
>>  LABEL                [a-zA-Z_][a-zA-Z0-9_]*
>>  STRING               \"([^\\"]|\\.)*\"
>> +CHAR_LITERAL '[^']+'|'\\''
>
> I think this should be
>        '([^']|\\')*'
>
> On the same grounds I described before.  That does mean
> eval_char_literal() will have to cope with the case of ''.

OK, I could also make it '([^']|\\')+'

That way the empty character literal wouldn't be allowed and it still
allows all of the cases we care about.

> [snip]
>> diff --git a/tests/char_literal.c b/tests/char_literal.c
>> new file mode 100644
>> index 0000000..150f2a0
>> --- /dev/null
>> +++ b/tests/char_literal.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + * libfdt - Flat Device Tree manipulation
>> + *   Testcase for character literals in dtc
>> + * Copyright (C) 2006 David Gibson, IBM Corporation.
>> + * Copyright (C) 2011 The Chromium Authors. All rights reserved.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public License
>> + * as published by the Free Software Foundation; either version 2.1 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <stdint.h>
>> +
>> +#include <fdt.h>
>> +#include <libfdt.h>
>> +
>> +#include "tests.h"
>> +#include "testdata.h"
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +     void *fdt;
>> +     uint32_t expected_cells[5];
>> +
>> +     expected_cells[0] = cpu_to_fdt32((unsigned char)TEST_CHAR1);
>> +     expected_cells[1] = cpu_to_fdt32((unsigned char)TEST_CHAR2);
>> +     expected_cells[2] = cpu_to_fdt32((unsigned char)TEST_CHAR3);
>> +     expected_cells[3] = cpu_to_fdt32((unsigned char)TEST_CHAR4);
>> +     expected_cells[4] = cpu_to_fdt32((unsigned char)TEST_CHAR5);
>> +
>> +     test_init(argc, argv);
>> +     fdt = load_blob_arg(argc, argv);
>> +
>> +     check_getprop(fdt, 0, "char-literal-cells",
>> +                   sizeof(expected_cells), expected_cells);
>> +
>> +     PASS();
>> +}
>
> Looks good, this is more or less exactly what I'd enivisaged for a
> testcase.

Glad to hear it.

Thanks,
    Anton

> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
>                                | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>