Hello, Sean.

2012/7/30 Sean Bartell <[email protected]>:
>> > However, many cases are not yet possible; for instance, USB descriptors
>> > can't use the length field because it includes its own length. This will
>> > be possible once scripts can get arbitrary subblobs.
>> I am not sure I follow. I thought that these subblobs would be useful
>> when the data contains some kind of pointers in them (e.g. that actual
>> data are at given offset). But here you have a bit different
>> situation, IMHO. If the descriptor is of unknown type (e.g. some
>> vendor specific), you want to skip next (.bLength - 2) bytes and
>> decode next descriptor. (Obviously, if you know the descriptor type,
>> you ignore the .bLength field.) I do not see how subblobs would help
>> you. What have I missed?
>
> First, when you know the descriptor type, I would still use the length
> field to get the right amount of data and then decode it. Your way is
> essentially equivalent, though. Second, I'm hoping to make all sorts of
> seeking possible with subblobs, so you could say "read a uint32le into
> .len, then go back and read .len raw bytes" to skip the unknown
> descriptor.
Thanks for the explanation. Makes sense to me now.

> I implemented "if" today, but documentation, examples, and "switch" will
> have to wait for tomorrow.
The functionality side is okay. I played with it a bit and it works fine.

However, you are still behind your (revised) schedule. And the amount
of commits (code) does not indicate any increased effort. At least
that is the way it looks to me...


During last days I also did a quick review of your new code and I have
a few comments.

In general, the code looks clean and tidy. Also, you are consistent
and the code is well structured.

Now, concretely: in parse_if, you jump into a conditional block. I
would consider that a very bad practice and actually I am surprised
that such jump does not produce any warning. Better is to go with the
common practice and put the error block at the end of the function.

In the same function, it looks to me that there is a leak in case
bithenge_if_transform() fails.

Another leak is in parse_expression() - if the
bithenge_const_expression() fails, you do not destroy the previously
created node.


Bye,
- Vojta


>
> Thanks,
> Sean
>
> _______________________________________________
> HelenOS-devel mailing list
> [email protected]
> http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel

Reply via email to