Hi David, Thanks for your comments.
On Sun, Sep 11, 2011 at 5:53 PM, David Gibson <[email protected]> wrote: > On Thu, Sep 08, 2011 at 10:44:40PM -0700, Simon Glass wrote: >> Hi David, >> >> A bit rushed as late here but I wanted to respond before your weekend. >> >> On Thu, Sep 8, 2011 at 9:49 PM, David Gibson >> <[email protected]> wrote: >> > On Thu, Sep 08, 2011 at 05:47:34AM -0700, Simon Glass wrote: > [snip] >> > 1) Use a subset of printf() specifiers. Despite your balking at that, >> > I don't really think it would be any harder than what you have now. >> > Last character is always the non-optional "format" specifier, anything >> > preceding it is modifiers. So for the integer formats "d", "x" and >> > "o", valid modifiers are size, either "hh" (1 byte), "h" (2 byte), "l" >> > (4 byte, default) or "ll" (8 byte). For the string format "s", no >> > modifiers are valid. >> >> OK, I like this option more. Particularly if you think I might get >> away with also supporting 'b' as a shortcut for 'hh'. > > Hrm, I have mixed feelings about that. "hh" is kind of obscure. And > as far as I can tell "b" is not used for anything else at present (at > least in glibc printf()). But there are so many letters with existing > meanings for printf() that redefining any makes me a little nervous. Well yes - I think I will support both hh and b and see how it looks. > >> > 2) Treat the size as a different parameter. So the -f option gives a >> > single format char (or instead use -s, -x, -d, .. options). Then have >> > a -s option for size (-s1 ... -s8) which is valid only with the >> > integer format options. >> >> More like what I had - have moved away from that and feel more >> comfortable in my new space. > > Ok. > >> > Either way does leave a couple of unanswered questions: >> > >> > A) What to do if the format doesn't consume the whole parameter. I >> > see two options: >> > A1) Just print as much as the format says, then stop >> > A2) Keep repeating the same format until the property is >> > consumed (note that this makes sense for "s" as well, and would be an >> > obvious way of handling the pretty common list-of-strings properties). >> >> Well I think you have to keep printing. This is not a true format >> string, but just a data type for each element. Again we have moved >> away from being able to say -f "The %d item is %d and has a flag word >> of %d". >> >> So no need to use -tddd if there are three cells - just -td is enough. > > Yes, I tend to agree. > >> > B) What to do when the property just doesn't fit into that format - >> > either it's length is not a multiple of the fixed integer size, or >> > it's not NUL-terminated for "s". Obviously some kind of error is in >> > order, but in case A2 above, do we *just* error, or print what we can >> > before giving up. >> >> Prefer to error since any output might be confusing. But I will take a >> look to make sure that is easy to do. > > It is trivial to check in advance for the existing types - for > integers just check if the property has length a multiple of the > integer size, for strings check if the last byte is '\0'. Well.. then > it depends a bit on how safe you want "s" to be - do you check for > reasonably printable characters first or not. How easy it is to > pre-check for possible future types is not neccessarily obvious of > course. Yes I have added a check for (length % size) == 0. For strings I don't think we should be picky so long as there is a NUL terminator. If you don't specify a type, the utility already tries to guess, and will fall back to integer or even byte if it finds the string has ctrl characters. When the user says -ts I think we need to try hard to obey. I can't think of any reason to store strange characters in a string property, but others might. I will send the V3 patch set in the next few days and we'll see how it looks. Regards, Simon > > -- > 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 > _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
