View Issue Details

IDProjectCategoryView StatusLast Update
0030764FPCFCLpublic2018-02-26 09:06
ReporterLacaK Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilitysometimes
Status closedResolutionfixed 
Product Version3.0.0 
Summary0030764: Utility for importing type libraries does not handle correctly function parameters of pointer types
DescriptionThere is packages\winunits-base\src\typelib.pas unit which is used for importing type libraries into pascal units. (procedure ImportTypelib)

However when we have function parameter(s) which should be passed by reference this is not taken into account and in output pascal unit is only "param: type" instead of "var param: type"

Example:
  function func(Type_:Integer;Value:WideString):Integer;dispid 33;
but it should be:
  function func(Type_:Integer;var Value:WideString):Integer;dispid 33;

Now look into typelib.pas in method TTypeLibImporter.interfacedeclaration
There are around line 631 handled function parameters.

TypeToString(TI,FD^.lprgelemdescParam[k].tdesc) returns 'PWideString' (in my case) but later because of test on line 634 bParamByRef is 'P' deleted.
Later on line 643 is tested FD^.lprgelemdescParam[k].paramdesc.wParamFlags but it is 0 (PARAMFLAG_NONE), so no 'var' nor 'out' is added.
Steps To ReproduceTry import any type library which published function/procedure with parameter of any pointer type (BSTR* , long*, etc.)
Additional Informationin Delphi is same type library imported with "var" before function parameter name.

https://msdn.microsoft.com/en-us/library/windows/desktop/aa367051(v=vs.85).aspx
"The [in] attribute is applied to a parameter by default when no directional parameter attribute is specified."

https://msdn.microsoft.com/en-us/library/cc237804.aspx
https://msdn.microsoft.com/en-us/library/windows/desktop/ms221019(v=vs.85).aspx
"PARAMFLAG_NONE: The behavior of the parameter is not specified."
Tagscom, importtl
Fixed in Revision38338
FPCOldBugId0
FPCTarget
Attached Files

Activities

Martok

2016-10-24 11:19

reporter   ~0095309

Do you have the IDL for these functions available? [in] Foo* is probably better translated as constref, as var implies [in,out].

LacaK

2016-10-24 13:26

developer   ~0095316

Last edited: 2016-10-25 07:13

View 2 revisions

I have only OCX. I can explore type library using OLEView (ITypeLib Viewer):
  // Generated .IDL file (by the OLE/COM Object Viewer)
  //
  // typelib filename: sc_x64_samlight_client_ctrl.ocx
  ...

"constref" gives sense, but Delphi generates "var" also ...

Attached patch, which fixes bug in my use case.

LacaK

2016-10-25 07:12

developer  

typelib.pas.diff (1,184 bytes)   
--- typelib.pas.ori	Thu Oct 20 14:46:27 2016
+++ typelib.pas	Mon Oct 24 13:29:57 2016
@@ -641,9 +641,10 @@
           sPar:='';
           if bParamByRef then
             case FD^.lprgelemdescParam[k].paramdesc.wParamFlags and (PARAMFLAG_FIN or PARAMFLAG_FOUT) of
-            PARAMFLAG_FIN or PARAMFLAG_FOUT:sPar:='var ';
-            PARAMFLAG_FOUT:sPar:='out ';
-            PARAMFLAG_FIN:sPar:='var '; //constref in safecall? TBD
+              PARAMFLAG_FIN or PARAMFLAG_FOUT: sPar:='var ';
+              PARAMFLAG_FOUT: sPar:='out ';
+              PARAMFLAG_NONE,              // [in] is default when no directional parameter attribute is specified
+              PARAMFLAG_FIN: sPar:='var '; // constref in safecall? TBD
             end;
           if not MakeValidId(GetName(k+1),sVarName) then
             AddToHeader('//  Warning: renamed parameter ''%s'' in %s.%s to ''%s''',[GetName(k+1),iname,sMethodName,sVarName],True);
@@ -1835,6 +1836,7 @@
     end;
   if (L<>'  ') then
     UnitSource.Add(L);
+  UnitSource.Add('');
   UnitSource.addStrings(InterfaceSection);
   UnitSource.addStrings(ImplementationSection);
   UnitSource.Add('end.');
typelib.pas.diff (1,184 bytes)   

Martok

2016-10-25 08:20

reporter   ~0095318

Regenerated IDL is fine too, as a formal language it's just less ambiguous than comparing to whatever Delphi generated (which is wrong more often than you'd think, learned that the hard way!).

I'm pretty sure [in] BSTR* Foo is meaningless for OLE. BSTR is always a reference (to a character: typedef WCHAR OLECHAR; typedef OLECHAR* BSTR), passing the address of that address and not specifying [out] as well seems very odd.

Some correct functions I found when quickly looking around classes I've imported in the past:
    HRESULT ConvertFromXML([in] BSTR XMLString);
    HRESULT ConvertToXML([out, retval] BSTR* pRetVal);

The correct imports for that are:
   procedure ConvertFromXML(XMLString:WideString);safecall;
   function ConvertToXML:WideString;safecall;
//or if you left out [oleautomation] on the interface:
   function ConvertToXML(out Ret:WideString):HRESULT;

Also, AFAIR Delphi has/had no constref, which also makes a significant amount of their shellapi imports (passing IIDs...) dubious at best. Some of ours, too *coughs*.

LacaK

2016-10-25 10:45

developer   ~0095320

So what is your suggestion related to patch ? Use "constref":
+ PARAMFLAG_NONE, // [in] is default when no directional parameter attribute is specified
+ PARAMFLAG_FIN: sPar:='constref '; // constref in safecall? TBD

Thaddy de Koning

2016-11-04 17:25

reporter   ~0095567

"but it should be:
  function func(Type_:Integer;var Value:WideString):Integer;dispid 33;"

No! it should be:
  function func(Type_:Integer;Value:PWideChar):Integer;dispid 33;

Reason is nil values are valid, so it should be a pointer type.

Martok

2016-11-04 17:37

reporter   ~0095569

Last edited: 2016-11-04 17:38

View 2 revisions

Sorry, forgot to monitor this issue...

That depends. Either we want pointer unwrapping or not. If we want [in] long * to be PLong, then everything is fine by default. Otherwise yes, we need constref/var/out depending on the direction (in,inout,out). Or only unwrap inout/out...

Needs verification of the generated code though, because I believe WideString gets auto-converted to PWideChar in this context and that probably doesn't work for PWideString/PPWideChar. Or does it?

Edit @Thaddy: The empty pascal-string converts to nil, so it's fine.

LacaK

2016-11-08 10:52

developer   ~0095651

Last edited: 2016-11-09 15:04

View 2 revisions

When I use Value: PWideChar or PPWideChar or PWideString I get error during compilation: "Error: Type is not automatable: "PWideChar""

So question is: should we use "var" as in typelib.pas.diff (Delphi compatible) or should I create new patch with "constref" ? (in my case both works)

Marco van de Voort

2018-02-25 15:56

manager   ~0106619

I enhanced the patch to also support XE3+ const [ref], and added a parameter --ref-style to importttl to configure it.

To get constref FPC style you need

importtl --ref-style constref <filename>

for XE3+ style

importtl --ref-style constrefdelphi <filename>

and VAR is still default

Issue History

Date Modified Username Field Change
2016-10-20 13:25 LacaK New Issue
2016-10-20 13:37 LacaK Tag Attached: importtl
2016-10-20 14:12 Marco van de Voort Tag Attached: com
2016-10-20 15:24 LacaK Description Updated View Revisions
2016-10-24 08:29 LacaK Summary Utility for importing type libraries does not handle correctly function parameter pointer types to WideString (at least) => Utility for importing type libraries does not handle correctly function parameters of pointer types
2016-10-24 08:29 LacaK Description Updated View Revisions
2016-10-24 08:29 LacaK Steps to Reproduce Updated View Revisions
2016-10-24 08:29 LacaK Additional Information Updated View Revisions
2016-10-24 11:19 Martok Note Added: 0095309
2016-10-24 13:26 LacaK Note Added: 0095316
2016-10-25 07:12 LacaK File Added: typelib.pas.diff
2016-10-25 07:13 LacaK Note Edited: 0095316 View Revisions
2016-10-25 08:20 Martok Note Added: 0095318
2016-10-25 10:45 LacaK Note Added: 0095320
2016-11-04 17:25 Thaddy de Koning Note Added: 0095567
2016-11-04 17:37 Martok Note Added: 0095569
2016-11-04 17:38 Martok Note Edited: 0095569 View Revisions
2016-11-08 10:52 LacaK Note Added: 0095651
2016-11-09 15:04 LacaK Note Edited: 0095651 View Revisions
2018-02-25 15:56 Marco van de Voort Fixed in Revision => 38338
2018-02-25 15:56 Marco van de Voort Note Added: 0106619
2018-02-25 15:56 Marco van de Voort Status new => resolved
2018-02-25 15:56 Marco van de Voort Resolution open => fixed
2018-02-25 15:56 Marco van de Voort Assigned To => Marco van de Voort
2018-02-26 09:06 LacaK Status resolved => closed