Hi Alex,
On 2009-04-24, at 04:41:23 -0400, Alexander Ost via RT wrote:
Show quoted text> Queue: Convert-Binary-C
> Ticket <URL:
https://rt.cpan.org/Ticket/Display.html?id=45130 >
>
> Hi Marcus,
>
> please excuse my late reply - I did some additional tests (0.74 works
> fine, thanks for that) and now I think I completely understood the
> implications of your last email...
>
> On Sat Apr 18 15:22:00 2009, mhx-perl@gmx.net wrote:
> > [...]
> >
> > Yes, that's exactly the one "shortcoming" that the docs talk about.
> > The Dimension tag doesn't affect compound layout.
>
> Ow... my initial understanding was that this affects just direct
> access to the compound element offsets. However, now I noticed that it
> also affects normal "straightforward" unpacking, since the data
> _after_ a variable size array is always read from the original,
> "static" offset.
>
> So currently it is not possible to correctly decode data for a
> compound like the one in the attached script...?
I think that greatly depends on the definition of "correctly". ;-)
The above code is not legal C, as the standard forbids a flexible
array member anywhere but at the end of a structure:
mhx@r2d2 ~ $ cat foo.c
struct S
{
int count;
int array[];
int foo;
};
mhx@r2d2 ~ $ cc -c foo.c
foo.c:4: error: flexible array member not at end of struct
Show quoted text> It outputs
>
> $ perl -w dimtest2.pl
> array mode yields:
> $VAR1 = {
> 'count' => '1',
> 'array' => [
> 2
> ],
> 'foo' => '2'
> };
> string mode yields:
> $VAR1 = {
> 'count' => '1',
> 'array' => [
> 2
> ],
> 'foo' => '2'
> };
>
> but correct output should be '1',[2],'3'.
Again, that depends on what you expect to get from a snippet
of illegal C code. Actually, to me, the output makes perfect
sense. The following is a valid piece of C code that is more
or less equivalent to your example:
struct S
{
int count;
struct {
int count2;
int array[];
} a;
int foo;
};
And in this case, for the C compiler, S.foo is actually an
alias for S.a.array[0]. As stated in a previous reply, the
flexible array member doesn't contribute to struct size.
I fully agree that if you actually tag a dimension to the
array (which ideally would turn that array from a flexible
array member into a fixed-size array), the behaviour should
be different and yes, I would in this case expect your output.
Show quoted text> > The warning is being issued when unpacking in scalar context.
> > This is because "unpack" believes that the struct is larger than it
> > would be based on the dimension from count. And that's because the
> > real
> > size isn't known until the data structure is unpacked. So, to
> > implement
> > your suggested behaviour, "unpack" would need to be changed to a two-
> > pass
> > algorithm. Thinking about all the various subtleties this behavourial
> > change implies (like making sizeof work consistently in more complex
> > scenarios, for example) makes my head hurt...
>
> I can imagine that this is a nightmare :-)
>
> However, I'm not sure that you really need a two-pass algorithm in
> general. As long as you're just interested in decoding a binary string
> completely from beginning to end, you could just maintain some sort of
> "dynamic component correction offset" which is increased on the
> fly. Whenever decoding of a dynamic component has been completed, this
> offset needs to be adapted. Subsequently, the offset has to be
> considered by all other decoding functions when accessing the input
> data.
You're probably correct. The logic that issues the "Data too short"
warning currently needs to know the size of a type before doing the
unpacking. But in case of dynamic sized types, that logic will most
probably have to change anyway.
Show quoted text> But then, I'm completely unaware of the inner workings of your
> package.... maybe this is much more complex then it seems now (I'll
> have a close look for sure :-)
Don't look too closely ;-) No, not because I fear you might dislike
the code, but because it's unlikely that I will do any large changes
to the currently released code base. My development version features
a completely new parser and AST generator, which will have a major
impact on the rest of the code (so major that currently nothing but
the new parser is working really well).
Show quoted text> > The best thing you can do is to use a flexible
> > array member in the compound. But even then unpack won't do the right
> > thing in list context.
>
> Yep... this will only prevent the "message too short error", because
> dynamic elements will be treated as zero size elements whenever
> accessing data that is behind them... correct?
Yes.
Show quoted text> > Sorry for not being very helpful this time.
>
> You were, really, even if there's no solution right now. I really
> appreciate your support.
>
> Just one minor question: maybe an explicit hint in the documentation
> would be useful, indicating that decoding any data that follows
> dynamic elements will not give the expected results. To me, this
> wasn't immediately clear from the documentation.
Ok, I'll keep that in mind for the next release.
Cheers,
Marcus
