Skip Menu |

This queue is for tickets about the Win32-API CPAN distribution.

Report information
The Basics
Id: 74578
Status: resolved
Worked: 1 hour (60 min)
Priority: 0/
Queue: Win32-API

People
Owner: cosimo [...] cpan.org
Requestors: DOUGW [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.64
Fixed in: (no value)



<p> When using Win32::API::Struct on 64bit, I get extraneous "x" (null bytes) in the pack/unpack templates. When I do a straight unpack without the x's, I get the data correctly. </p> <p> Or if I remove the 'align' logic in Win32::API::Struct::getPack() and getUnpack(), I also get a correct result. Is this a bug or is there something else I should be doing? </p> <pre> $Win32::API::DEBUG = 1; Win32::API::Struct->typedef('DLXSTRUCT', qw( CHAR VarName[12]; LONG32 StartDate; LONG32 EndDate; INT32 NumberObs; INT32 Frequency; ULONG32 DateTimeMod; INT32 Magnitude; INT32 DecPrecision; INT32 DifType; INT32 AggType; CHAR DataType[8]; CHAR Group[4]; CHAR Source[6]; CHAR Geography1[8]; CHAR Geography2[8]; CHAR Descriptor[82]; CHAR ShortSource[10]; CHAR LongSource[70]; )) or die "Err"; # 'PPS' when using the Struct above # 'PPP' when using the buffer below my $GetInfo = Win32::API->new( # $dll, 'DLXGetInfo', 'PPP', 'I', $dll, 'DLXGetInfo', 'PPS', 'I', ) or die "Err: $!"; # When using 'PPP' above #my $dlx = " " x 246; # Character fields missing first 3 characters, number fields are garbage my $info_result = $get_info->Call('', $field, $dlx); # Debug output: # (PM)Struct::getPack: DLXSTRUCT(buffer) = pack(a12xlxxliiLiiiia8a4a6a8a8a82a10a71, 48) # (PM)Struct::getUnpack(DLXSTRUCT): unpack(Z12xlxxliiLiiiiZ8Z4Z6Z8Z8Z82Z10Z71, ...field names) # For 'PPP' call above # Everything ok #my ($name, $start, $end, $n_obs, $freq, $dt_mod, $mag, $prec, # $diftype, $agg_type, $data_type, $group, $source, $geo1, $geo2, $desc, $short_source, $long_source) # = unpack('Z12 l l i i L i i i i Z8 Z4 Z6 Z8 Z8 Z82 Z10 Z70', $dlx); </pre>
Hi Doug, thanks for filing this. Unfortunately, packing and unpacking of structs is buggy, especially in 64 bits builds. What do to about it? Not sure, other than CC'ing libwin32@perl.org as you did.
On Wed Feb 01 02:15:26 2012, COSIMO wrote: Show quoted text
> Hi Doug, thanks for filing this. > > What do to about it? > Not sure, other than CC'ing libwin32@perl.org as you did.
Cool, I didn't know RT cc'd the mailing list (maybe someone else will know what to do). What I've done is hack Struct.pm getPack() and getUnpack(): in getPack(): $type_align = (($packed_size + $type_size) % $type_size); # $packing .= "x" x $type_align . $type; $packing .= $type; $packed_size += $type_size; # $packed_size += $type_size + $type_align; } in getUnpack(): $type_align = (($packed_size + $type_size) % $type_size); # $packing .= "x" x $type_align . $type; $packing .= $type; # $packed_size += $type_size + $type_align; $packed_size += $type_size; push(@items, $name); } Perhaps if this change were wrapped in is_64bit() type logic, then it would work everywhere? I don't know enough about this to really know for sure, and I'm not even sure if I haven't broken something else. And if this change is headed in the right direction, there should probably be some changes to the align() and sizeof() methods.
CC: libwin32 [...] perl.org, Douglas Wilson via RT <bug-Win32-API [...] rt.cpan.org>
Subject: Re: [rt.cpan.org #74578] Win32::API::Struct not aligned on 64 bit
Date: Thu, 2 Feb 2012 08:26:55 -0600
To: Cosimo Streppone <cosimo [...] streppone.it>
From: Reini Urban <rurban [...] cpanel.net>
Am 01.02.2012 um 11:00 schrieb Cosimo Streppone: Show quoted text
> This is RT#74578 about Win32::API structs packing/unpacking. > Can you help us? > > ------- Forwarded message ------- > From: "Douglas Wilson via RT" <bug-Win32-API@rt.cpan.org> > Subject: [rt.cpan.org #74578] Win32::API::Struct not aligned on 64 bit > Date: Wed, 01 Feb 2012 17:27:37 +0100 > > Queue: Win32-API > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=74578 > > > On Wed Feb 01 02:15:26 2012, COSIMO wrote:
>> Hi Doug, thanks for filing this. >> >> What do to about it? >> Not sure, other than CC'ing libwin32@perl.org as you did.
> > Cool, I didn't know RT cc'd the mailing list (maybe someone else will > know what to do). What I've done is hack Struct.pm getPack() and > getUnpack(): > > in getPack(): > $type_align = (($packed_size + $type_size) % $type_size); > # $packing .= "x" x $type_align . $type; > $packing .= $type; > $packed_size += $type_size; > # $packed_size += $type_size + $type_align; > } > > in getUnpack(): > $type_align = (($packed_size + $type_size) % $type_size); > # $packing .= "x" x $type_align . $type; > $packing .= $type; > # $packed_size += $type_size + $type_align; > $packed_size += $type_size; > > push(@items, $name); > } >
I'm pretty sure deleting type_align on 64bit is wrong. There must be an error in the size calculation for 8 byte sizes, which will result in the needed type_align=0. I have got no win64 bit vm (yet), but I will try to study it today. Show quoted text
> Perhaps if this change were wrapped in is_64bit() type logic, then it > would work everywhere? I don't know enough about this to really know for > sure, and I'm not even sure if I haven't broken something else. And if > this change is headed in the right direction, there should probably be > some changes to the align() and sizeof() methods.
Reini
FYI, I just realized that while I'm using a 64-bit machine, the perl I was using is a 32-bit perl. Under a 64-bit perl, the dll I'm trying to use gives "%1 is not a valid Win32 Application". That said, Win32::API::getPack() and the calls to it are wrong. It returns a scalar, array, scalar, but in the calls to it, the last scalar will be slurped into the array. Easiest thing to do is probably rearrange the return values and return both scalars before the array, and adjust all the calls to the function.
On Fri Feb 03 18:35:45 2012, DOUGW wrote: Show quoted text
> > That said, Win32::API::getPack() and the calls to it are wrong. It
I mean Win32::API::Struct::getUnpack. Sorry for the confusion.
On Fri Feb 03 18:36:54 2012, DOUGW wrote: Show quoted text
> On Fri Feb 03 18:35:45 2012, DOUGW wrote:
> > > > That said, Win32::API::getPack() and the calls to it are wrong. It
> > I mean Win32::API::Struct::getUnpack. Sorry for the confusion.
I have a patch for this pack/unpack issue (the align is still off). I'm using the struct as the basis for the unpack via getUnpack() to get the size and unpack template. Unfortunately, I've switched from AS perl to Strawberry, and upgraded to 5.14. Now the program works ok, but I get "Free to wrong pool" when the program exits. I'll try re-installing Win32::API. Or I'm going to see what happens when I use AS perl again.
Subject: Struct_diff.txt
--- Struct_orig.pm 2012-02-03 15:39:35 -0800 +++ Struct.pm 2012-02-03 15:52:38 -0800 @@ -200,9 +200,10 @@ $packing .= $subpacking; $packed_size += $subpacksize; } else { + my $repeat = 1; if($type =~ /\w\*(\d+)/) { - my $size = $1; - $type = "a$size"; + $repeat = $1; + $type = "a$repeat"; } DEBUG "(PM)Struct::getPack($self->{__typedef__}) ++ $type\n"; @@ -215,9 +216,9 @@ } push(@recipients, $self); $type_size = Win32::API::Type::sizeof($orig); - $type_align = (($packed_size + $type_size) % $type_size); + $type_align = (($packed_size + ($type_size*$repeat)) % $type_size); $packing .= "x" x $type_align . $type; - $packed_size += $type_size + $type_align; + $packed_size += ($type_size*$repeat) + $type_align; } } @@ -253,32 +254,33 @@ foreach my $member (@{ $self->{typedef} }) { ($name, $type, $orig) = @$member; if($type eq '>') { - my($subpacking, @subitems, $subpacksize) = $self->{$name}->getUnpack(); + my($subpacking, $subpacksize, @subitems) = $self->{$name}->getUnpack(); DEBUG "(PM)Struct::getUnpack($self->{__typedef__}) ++ $subpacking\n"; $packing .= $subpacking; $packed_size += $subpacksize; push(@items, @subitems); } else { + my $repeat = 1; if($type =~ /\w\*(\d+)/) { - my $size = $1; - $type = "Z$size"; + my $repeat = $1; + $type = "Z$repeat"; } DEBUG "(PM)Struct::getUnpack($self->{__typedef__}) ++ $type\n"; $type_size = Win32::API::Type::sizeof($orig); - $type_align = (($packed_size + $type_size) % $type_size); + $type_align = (($packed_size + ( $type_size * $repeat )) % $type_size); $packing .= "x" x $type_align . $type; - $packed_size += $type_size + $type_align; + $packed_size += ( $type_size * $repeat ) + $type_align; push(@items, $name); } } DEBUG "(PM)Struct::getUnpack($self->{__typedef__}): unpack($packing, @items)\n"; - return($packing, @items, $packed_size); + return($packing, $packed_size, @items); } sub Unpack { my $self = shift; - my($packing, @items) = $self->getUnpack(); + my($packing, undef, @items) = $self->getUnpack(); my @itemvalue = unpack($packing, $self->{buffer}); DEBUG "(PM)Struct::Unpack: unpack($packing, buffer) = @itemvalue\n"; foreach my $i (0..$#items) {
On Fri Feb 03 19:09:56 2012, DOUGW wrote: Show quoted text
> On Fri Feb 03 18:36:54 2012, DOUGW wrote:
> > On Fri Feb 03 18:35:45 2012, DOUGW wrote:
> > > > > > That said, Win32::API::getPack() and the calls to it are wrong. It
> > > > I mean Win32::API::Struct::getUnpack. Sorry for the confusion.
> > I have a patch for this pack/unpack issue (the align is still off).
Grr, the second "my $repeat" in the patch should just be "$repeat". No more free to wrong pool error. Still have same old align problem.
CC: libwin32 [...] perl.org
Subject: Re: [rt.cpan.org #74578] Win32::API::Struct not aligned on 64 bit
Date: Fri, 3 Feb 2012 19:51:15 -0600
To: bug-Win32-API [...] rt.cpan.org
From: Reini Urban <rurban [...] x-ray.at>
Patch looks good. Aldo wanted to have that repeat bit fixed for a long time. Thanks. Now if you can repro the w64 align problem and send the output with debugging info, please? On Fri, Feb 3, 2012 at 6:36 PM, Douglas Wilson via RT <bug-Win32-API@rt.cpan.org> wrote: Show quoted text
> Fri Feb 03 19:36:37 2012: Request 74578 was acted upon. > Transaction: Correspondence added by DOUGW >       Queue: Win32-API >     Subject: Win32::API::Struct not aligned on 64 bit >   Broken in: 0.64 >    Severity: Normal >       Owner: Nobody >  Requestors: DOUGW@cpan.org >      Status: open >  Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=74578 > > > > On Fri Feb 03 19:09:56 2012, DOUGW wrote:
>> On Fri Feb 03 18:36:54 2012, DOUGW wrote:
>> > On Fri Feb 03 18:35:45 2012, DOUGW wrote:
>> > > >> > > That said, Win32::API::getPack() and the calls to it are wrong. It
>> > >> > I mean Win32::API::Struct::getUnpack. Sorry for the confusion.
>> >> I have a patch for this pack/unpack issue (the align is still off).
> > Grr, the second "my $repeat" in the patch should just be "$repeat". No > more free to wrong pool error. Still have same old align problem.
-- Reini Urban http://cpanel.net/   http://www.perl-compiler.org/
On Fri Feb 03 20:51:26 2012, rurban@x-ray.at wrote: Show quoted text
> Patch looks good. Aldo wanted to have that repeat bit fixed for a long
time. Show quoted text
> Thanks. > > Now if you can repro the w64 align problem and send the output with > debugging info, please?
Like I said before, I don't think 64-bit had anything to do with this bug, as I was using a 32-bit perl and dll. I've been reading up on Win32 alignment and finally completely understand what needs to be done. My test structure was suppose to start off with a char[10] and I thought it was just a typo in the docs...now I see that it should have "xx" after it in the pack/unpack. Attached is an updated patch, which correctly fixes the repeat logic, and also I believe fixes the alignment of the elements in the Struct (on closer look, the repeat logic is why the alignments were wrong). It looks good just from calling getPack and getUnpack. I'll test it with the actual dll on Monday.
Subject: Struct.diff.txt
--- Struct.pm 2012-02-03 15:39:35 -0800 +++ Struct.pm 2012-02-03 15:52:38 -0800 @@ -200,9 +200,10 @@ $packing .= $subpacking; $packed_size += $subpacksize; } else { + my $repeat = 1; if($type =~ /\w\*(\d+)/) { - my $size = $1; - $type = "a$size"; + $repeat = $1; + $type = "a$repeat"; } DEBUG "(PM)Struct::getPack($self->{__typedef__}) ++ $type\n"; @@ -215,9 +216,9 @@ } push(@recipients, $self); $type_size = Win32::API::Type::sizeof($orig); - $type_align = (($packed_size + $type_size) % $type_size); + $type_align = (($packed_size + $type_size) % $type_size); $packing .= "x" x $type_align . $type; - $packed_size += $type_size + $type_align; + $packed_size += ($type_size*$repeat) + $type_align; } } @@ -253,32 +254,33 @@ foreach my $member (@{ $self->{typedef} }) { ($name, $type, $orig) = @$member; if($type eq '>') { - my($subpacking, @subitems, $subpacksize) = $self->{$name}->getUnpack(); + my($subpacking, $subpacksize, @subitems) = $self->{$name}->getUnpack(); DEBUG "(PM)Struct::getUnpack($self->{__typedef__}) ++ $subpacking\n"; $packing .= $subpacking; $packed_size += $subpacksize; push(@items, @subitems); } else { + my $repeat = 1; if($type =~ /\w\*(\d+)/) { - my $size = $1; - $type = "Z$size"; + $repeat = $1; + $type = "Z$repeat"; } DEBUG "(PM)Struct::getUnpack($self->{__typedef__}) ++ $type\n"; $type_size = Win32::API::Type::sizeof($orig); - $type_align = (($packed_size + $type_size) % $type_size); + $type_align = (($packed_size + $type_size) % $type_size); $packing .= "x" x $type_align . $type; - $packed_size += $type_size + $type_align; + $packed_size += ( $type_size * $repeat ) + $type_align; push(@items, $name); } } DEBUG "(PM)Struct::getUnpack($self->{__typedef__}): unpack($packing, @items)\n"; - return($packing, @items, $packed_size); + return($packing, $packed_size, @items); } sub Unpack { my $self = shift; - my($packing, @items) = $self->getUnpack(); + my($packing, undef, @items) = $self->getUnpack(); my @itemvalue = unpack($packing, $self->{buffer}); DEBUG "(PM)Struct::Unpack: unpack($packing, buffer) = @itemvalue\n"; foreach my $i (0..$#items) {
On Sun Feb 05 14:10:04 2012, DOUGW wrote: Show quoted text
> > alignments were wrong). It looks good just from calling getPack and > getUnpack. I'll test it with the actual dll on Monday.
Tested and works fine. Sorry for the w64 red herring. I think I assumed it must have been a w64 issue because code that had been around this long couldn't have these sort of bugs :-)
On Mon Feb 06 11:34:46 2012, DOUGW wrote: Show quoted text
> On Sun Feb 05 14:10:04 2012, DOUGW wrote: > > Tested and works fine. Sorry for the w64 red herring. I think I assumed > it must have been a w64 issue because code that had been around this > long couldn't have these sort of bugs :-)
Also, it would be nice if all of the .pm files were run through perltidy (preferably in a release with no other changes). Mixing tabs and spaces in the same line of indentation was really throwing me off at times.
Subject: Re: [rt.cpan.org #74578] Win32::API::Struct alignment wrong
Date: Fri, 10 Feb 2012 12:42:41 +0100
To: bug-Win32-API [...] rt.cpan.org
From: "Cosimo Streppone" <cosimo [...] streppone.it>
On Sun, 05 Feb 2012 20:10:05 +0100, Douglas Wilson via RT <bug-Win32-API@rt.cpan.org> wrote: Show quoted text
> On Fri Feb 03 20:51:26 2012, rurban@x-ray.at wrote:
>> Patch looks good. Aldo wanted to have that repeat bit fixed for a long >> time. Thanks. >> >> Now if you can repro the w64 align problem and send the output with >> debugging info, please?
> > Like I said before, I don't think 64-bit had anything to do with this > bug, as I was using a 32-bit perl and dll. I've been reading up on Win32 > alignment and finally completely understand what needs to be done. My > test structure was suppose to start off with a char[10] and I thought it > was just a typo in the docs...now I see that it should have "xx" after > it in the pack/unpack. Attached is an updated patch, which correctly > fixes the repeat logic, and also I believe fixes the alignment of the > elements in the Struct (on closer look, the repeat logic is why the > alignments were wrong). It looks good just from calling getPack and > getUnpack. I'll test it with the actual dll on Monday.
Thanks everyone involved, Douglas and Reini. I'm going to release this patch to CPAN during the weekend. -- Cosimo
On Fri Feb 10 06:43:35 2012, cosimo@streppone.it wrote: Show quoted text
> > I'm going to release this patch to CPAN > during the weekend. >
I see you perltidy'd everything. Here's a patch against version 0.65 in case you want to just use the 'patch' command.
Subject: Struct.pm.diff.txt
--- Struct.pm Sun Feb 12 06:39:26 2012 +++ Struct.pm Mon Feb 13 12:22:16 2012 @@ -212,9 +212,10 @@ $packed_size += $subpacksize; } else { + my $repeat = 1; if ($type =~ /\w\*(\d+)/) { - my $size = $1; - $type = "a$size"; + $repeat = $1; + $type = "a$repeat"; } DEBUG "(PM)Struct::getPack($self->{__typedef__}) ++ $type\n"; @@ -230,7 +231,7 @@ $type_size = Win32::API::Type::sizeof($orig); $type_align = (($packed_size + $type_size) % $type_size); $packing .= "x" x $type_align . $type; - $packed_size += $type_size + $type_align; + $packed_size += ( $type_size * $repeat ) + $type_align; } } @@ -267,7 +268,7 @@ foreach my $member (@{$self->{typedef}}) { ($name, $type, $orig) = @$member; if ($type eq '>') { - my ($subpacking, @subitems, $subpacksize) = $self->{$name}->getUnpack(); + my ($subpacking, $subpacksize, @subitems) = $self->{$name}->getUnpack(); DEBUG "(PM)Struct::getUnpack($self->{__typedef__}) ++ $subpacking\n"; $packing .= $subpacking; $packed_size += $subpacksize; @@ -274,26 +275,27 @@ push(@items, @subitems); } else { + my $repeat = 1; if ($type =~ /\w\*(\d+)/) { - my $size = $1; - $type = "Z$size"; + $repeat = $1; + $type = "Z$repeat"; } DEBUG "(PM)Struct::getUnpack($self->{__typedef__}) ++ $type\n"; $type_size = Win32::API::Type::sizeof($orig); $type_align = (($packed_size + $type_size) % $type_size); $packing .= "x" x $type_align . $type; - $packed_size += $type_size + $type_align; + $packed_size += ( $type_size * $repeat ) + $type_align; push(@items, $name); } } DEBUG "(PM)Struct::getUnpack($self->{__typedef__}): unpack($packing, @items)\n"; - return ($packing, @items, $packed_size); + return ($packing, $packed_size, @items); } sub Unpack { my $self = shift; - my ($packing, @items) = $self->getUnpack(); + my ($packing, undef, @items) = $self->getUnpack(); my @itemvalue = unpack($packing, $self->{buffer}); DEBUG "(PM)Struct::Unpack: unpack($packing, buffer) = @itemvalue\n"; foreach my $i (0 .. $#items) {
RT-Send-CC: libwin32 [...] perl.org, cosimo [...] streppone.it, rurban [...] x-ray.at
Thanks Douglas and Reini. I included Douglas fix on structs alignment in v0.66 and uploaded it right now to the CPAN, also pushed it to github (github.com/cosimo/perl5-win32-api). I'm trying to get in touch with #msopensource again to get a Windows test machine. I'm feel embarrassed in maintaining Win32::API without a test installation of Windows where to test code. I couldn't put together a test case for the new bug fix. I'm trying to do something about it... In the meantime, enjoy 0.66 on CPAN. If there's problems, please let me know.
Subject: Win32-API-0.66.tar.gz
Download Win32-API-0.66.tar.gz
application/x-gzip 337.2k

Message body not shown because it is not plain text.