[Bug 24759] New: safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Summary: safearray axis in reversed order
           Product: Wine
           Version: unspecified
          Platform: x86
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: oleaut32
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

The api SafeArrayCreate expected a vector holding the shape
of array, eg. foo[3,4], the vector should be {3,4}. But what 
I found was that wine interprets this vector in the reversed 
order of what a real window would do. Hence one need to supply
a vector of {4,3} to wine to create the correct shape.

Also related to this, other api that need axis number are also
affected. eg. SafeArrayGetLBound/SafeArrayGetUBound
SafeArrayGetLBound ,1,   will retrieve lbound of the second axis
SafeArrayGetLBound ,2,   will retrieve lbound of the first axis

steps to reproduce problem,
create a safearray of shape [3,4] of integer from 0 to 11
open excel via ole automation and get a range of cell of the same shape
put the variant of the safearray into range
cleanup ole stuff
use excel or other spreadsheet programs to inspect the result.
expected output is a block of cells
0  1  2  3
4  5  6  7
8  9 10 11

actual output:
numbers not in this order and a column of cells with content <n/a>

repeat the test using real window and to confirm result.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #1 from Nikolay Sivov  2010-10-16 03:48:56 CDT ---
Please write a small C program to reproduce that uses only oleaut32 api
directly without Excel or whatever.

What Wine version by the way?

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #2 from bill lam  2010-10-16 04:17:59 CDT ---
I compiled wine from current git. os debian squeeze amd64

I think the error cannot manifest itself unless an external 
program that can be used to view result, because for a round
trip write then read, it passes two reversal and the net
result is ok.

I directly call ole api but I've never used C to do
it. I can try but could you show me a template of makefile and
a simple c program to call ole stuff?

I program in J and the stuff to create safearray is some 
ascii soup like this.

if. 0= sa=. SafeArrayCreate x ; (#$y) ; , (|.^:IFWINEBUG $y),.0 do.
  0 return.
end.
if. 0~: #,y do.
  p=. ,2-2    
  if. S_OK~: hr=. SafeArrayAccessData sa ; p do.
    SafeArrayDestroy sa
    0 return.
  end.
  if. (VT_UI1,VT_I1) e.~ x do.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unspecified                 |1.3.5

--- Comment #3 from Dmitry Timoshkov  2010-10-16 06:54:06 CDT ---
Please provide a test case in C, which could be easily compiled and run.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #4 from Nikolay Sivov  2010-10-16 07:06:52 CDT ---
(In reply to comment #2)
> I directly call ole api but I've never used C to do
> it. I can try but could you show me a template of makefile and
> a simple c program to call ole stuff?

You could find some examples in wine tree at dlls/oleaut32/tests, actually
there's a place to finally put a test in.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #5 from bill lam  2010-10-16 09:35:59 CDT ---
I cannot figure how to write a c program from them. C is 
not my expertise. Neither can I see those tests actually
write to an excel file or other external programs for validation.
If I am correct there was no validation test for the order
of axis of safearray.

But I found this commit 06fcfda9fff76f7f3d  in 2006 reverse
the order of axis, but there was no comment to state 
why the author wanted it or what bug it was supposed to fix.
I revert that patch and compile again. Now the safearray 
behavior under wine agrees with that under real window.

I can post the complete source of my J scripts if you are
interested.   Thanks.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #6 from Nikolay Sivov  2010-10-16 09:50:11 CDT ---
(In reply to comment #5)
> But I found this commit 06fcfda9fff76f7f3d  in 2006 reverse
> the order of axis, but there was no comment to state 
> why the author wanted it or what bug it was supposed to fix.
> I revert that patch and compile again. Now the safearray 
> behavior under wine agrees with that under real window.

If you revert only implementation part of commit do the tests fail?

What we need now is a simple call sequence of API calls that shows a
difference, in pseudocode please or just a numbered list cause I find this
scripting syntax unreadable.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #7 from Nikolay Sivov  2010-10-16 13:26:55 CDT ---
Created an attachment (id=31309)
 --> (http://bugs.winehq.org/attachment.cgi?id=31309)
test

Does attached test have all steps you're talking about? It doesn't fail running
with Wine.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #8 from bill lam  2010-10-16 19:32:33 CDT ---
The safearray created in test was in fact incorrect, 
but the safearraygetubound/safearraygetlbound were also
incorrect in the same manner. error cancel error hence
the result appeared to be correct. What is needed is a
third party program that will use the safearray to validate
the safearray created.

It began when someone asked in forum

In studying OLE Automation I found an example of programming 
via C# that appears to write an array directly to a spreadsheet 
(see code fragment below):
         // Create an array to multiple values at once.
                 string[,] saNames = new string[5,2];

saNames[ 0, 0] = "John";
                 saNames[ 0, 1] = "Smith";
                 saNames[ 1, 0] = "Tom";
                 saNames[ 1, 1] = "Brown";
                 saNames[ 2, 0] = "Sue";
                 saNames[ 2, 1] = "Thomas";
                 saNames[ 3, 0] = "Jane";
                 saNames[ 3, 1] = "Jones";
                 saNames[ 4, 0] = "Adam";
                 saNames[ 4, 1] = "Johnson";

//Fill A2:B6 with an array of values (First and Last Names).
                 oSheet.get_Range("A2", "B6").Value2 = saNames;

After some iterations, I got a way to do using safearray with a test
of pseudocode like,

NB. base is idispatch interface
  base=. createstddispatch 'Excel.Application'
NB. wb is workbooks property of base
  wb=. base.workbooks
NB. open method to open an excel file
  wb.open '/path/to/an_existing_excel_file'
  awb=. base.activeworkbook
  aws=. awb.worksheets  1     NB. sheet 1-base
NB. write number using safearray
NB. call method range of aws object for get block of shape of 3,4
  range=. aws.range  'C12:F14'
NB. create a safearray of integer 
NB. 0 1 2 3
NB. 4 5 6 7
NB. 8 9 10 11
NB. and wrap it inside a variant for ole automation use.
  sa=. safearray of shape (3,4) and values 0,1,2,3,4,5,6,7,8,9,10,11
  range.value2=. sa
  free sa
NB. read safearray for validation
  debugprint range.value2
NB. save and cleanup
  base.DisplayAlerts=. false
  awb.save
  awb.close
  base.quit

open the excel file, you will see a block like this,
0 5 10 #n/a
4 9 3  #n/a
8 2 7  #n/a

the last column was not initialised and the order of number in cells shuffled.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #9 from bill lam  2010-10-16 22:24:40 CDT ---
I'm not sure if this can be used. The structure of safearray is

tagSAFEARRAY {
  unsigned short cDims;
  unsigned short fFeatures;
  unsigned short cbElements;
  unsigned short cLocks;
  unsigned long handle;
  void HUGEP* pvData;
  SAFEARRAYBOUND rgsabound[1];
}

Under wine, create a rank-2 safearray using safearraycreate and 
then read the memory content of rgsabound. Re-run again using
real window to compare the content of rgsabound.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31309|0                           |1
        is obsolete|                            |

--- Comment #10 from Nikolay Sivov  2010-10-17 07:22:04 CDT ---
Created an attachment (id=31331)
 --> (http://bugs.winehq.org/attachment.cgi?id=31331)
test2

(In reply to comment #9)
> Under wine, create a rank-2 safearray using safearraycreate and 
> then read the memory content of rgsabound. Re-run again using
> real window to compare the content of rgsabound.

It should be stored in opposite order. See a test.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #11 from bill lam  2010-10-17 10:01:48 CDT ---
If the storage of safearray is correct then perhaps it
is the idispatch part that use this safearray had not 
been updated in the same patch.

Can you try my test pseudocode in c or other languages?

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #12 from Nikolay Sivov  2010-10-17 11:06:35 CDT ---
Created an attachment (id=31336)
 --> (http://bugs.winehq.org/attachment.cgi?id=31336)
patch

I don't really know typelib code, but this is the only suspicious place I
found. Please try this patch.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #13 from bill lam  2010-10-17 18:57:29 CDT ---
It did not fix the problem after applying your patch. Same
erraneous output as before.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #14 from bill lam  2010-10-17 19:28:09 CDT ---
I suspect the bug is inside usemarshal.c

wiresab = (SAFEARRAYBOUND *)Buffer;
    Buffer += sizeof(wiresab[0]) * wiresa->cDims;

if(vt)
        *ppsa = SafeArrayCreateEx(vt, wiresa->cDims, wiresab, NULL);

where Buffer is (I supposed) the internal record of safearray, if it
is the case then, the axis specification in wiresab will be in the
reversed order.  However SafeArrayCreate expects wiresab hold axis
dimension in the normal order. The patch in year 2006 only change
the storage of safearray but did not modify this part.

IMO what should be done is to create a vector from Buffer that holds 
the axis dimension in normal order for the consumption of 
safearraycreate. Can you write a patch for this?

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #15 from Nikolay Sivov  2010-10-18 04:22:12 CDT ---
Could be a case, yes. I'll test marshal part first (LPSAFEARRAY_UserMarshal)
probably we marshal in a wrong format in a first place.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31331|0                           |1
        is obsolete|                            |
  Attachment #31336|0                           |1
        is obsolete|                            |

--- Comment #16 from Nikolay Sivov  2010-10-18 17:39:51 CDT ---
Created an attachment (id=31370)
 --> (http://bugs.winehq.org/attachment.cgi?id=31370)
patch

Confirming. Bounds data is not marshalled properly, that's what is left unfixed
after that commit you mentioned.

[Bug 24759] safearray axis in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
         AssignedTo|[e-mail]        |[e-mail]
     Ever Confirmed|0                           |1

--- Comment #17 from Nikolay Sivov  2010-10-18 17:40:50 CDT ---
Confirming.

[Bug 24759] SAFEARRAY bounds data is marshalled in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|safearray axis in reversed  |SAFEARRAY bounds data is
                   |order                       |marshalled in reversed
                   |                            |order

[Bug 24759] SAFEARRAY bounds data is marshalled in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #18 from bill lam  2010-10-18 19:36:42 CDT ---
I tried this patch (without your previous patch) and it ran 
ok for the test case that I reported.

Thank you very much!

[Bug 24759] SAFEARRAY bounds data is marshalled in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

--- Comment #19 from Nikolay Sivov  2010-10-19 01:29:49 CDT ---
Good, thanks for report.

[Bug 24759] SAFEARRAY bounds data is marshalled in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #20 from Nikolay Sivov  2010-10-19 13:16:59 CDT ---
Fixed by commit b08d3d2656746cfd78be59cea2e06abd178a1ddc.

[Bug 24759] SAFEARRAY bounds data is marshalled in reversed order

http://bugs.winehq.org/show_bug.cgi?id=24759

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #21 from Alexandre Julliard  2010-10-29 12:55:56 CDT ---
Closing bugs fixed in 1.3.6.

[Bug 24759] SAFEARRAY bounds data is marshalled in reversed order

https://bugs.winehq.org/show_bug.cgi?id=24759

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[e-mail]        |[e-mail]