FWD » Core Development » Base Language

Provide a replacement for the Progress 4GL native API registration (PROCEDURE EXTERNAL) and calling mechanism. Utilize the native dynamic linking support (e.g. dlopen on Linux) to provide the same feature set. The calling mechanism will have to handle all the type mapping and parameter/return value semantics as needed. Conversion will be a pretty straightforward replacement, most work is in the runtime.

The conversion approach: generate a class for each DLL being called, the class should have static methods representing the API. Test if there is a real need for the PROCEDURE EXTERNAL call. For example, doe the library get loaded at that time or is it lazy? It is possible that the call may fail at that time if the library or API doesn't exist. These would be reasons to leave behind an equivalent. If possible, eliminate it. Either way, the registration details can be hidden inside the helper class implementation. The only question is whether the API call itself can do a lazy registration.

The runtime will need a JNI module (with implementations for each platform) to expose the dynamic loading/call features.

Please look at #1920-41 and the related prior notes for some very useful information regarding native procedures.

The following need to be supported:

PROCEDURE procname EXTERNAL "dllname":
DEFINE <type> PARAMETER ...
END PROCEDURE.

RUN procname_defined_via_procedure_external (parameters).

In addition to the normal data type usage, LONG, SHORT and BYTE types are in use as parameter types. Since one cannot create a variable of those types, I would expect that we would handle the type conversion inside the runtime. So, the types should be limited to signature definitions and should not need wrapper classes in the runtime (e.g. character).

INPUT, INPUT-OUTPUT, OUTPUT and RETURN parameter types are all in use.

I am assuming that the #1920 and #1921 already provide sufficient conversion support for the procedure handle attributes/methods that can be used for native library procedures (e.g. GET-SIGNATURE). If not, please detail here what else is needed.

The runtime stubs can be put into ControlFlowOps.

All native procedure calls convert the same way as internal (or external) procedures. The fact that the target is a native procedure is hidden in the runtime (as the conversion code can't decide if the target is a native procedure). The changes for #1920 already add all the native procedure clauses to the name_map.xml file.
The only known issues not supported at this time are full-parameter type support - see this note from #1920-41

OK, so the only remaining questions:

1. What do we do with the PROCEDURE EXTERNAL calls for their purpose of registration? I understand that you are already pulling the signature info into the name_map. Do you propose to hide the registration in the runtime? I presume it would be done lazily at the time of the first call. If so, then I guess there may be no need for any other conversion change for PROCEDURE EXTERNAL.

2. Do we handle PROCEDURE EXTERNAL PERSISTENT? If the runtime is hiding this, then it seems that we would need to remember that the registration would be PERSISTENT in the name map.

3, We would still need to implement a RELEASE EXTERNAL as a converted call.

4. I assume all the parameter conversion can be hidden in the runtime. Since the calling code cannot actually pass the native types, those are just for the runtime to know how to transform the data to match the underlying platform's API.

1. What do we do with the PROCEDURE EXTERNAL calls for their purpose of registration? I understand that you are already pulling the signature info into the name_map. Do you propose to hide the registration in the runtime? I presume it would be done lazily at the time of the first call. If so, then I guess there may be no need for any other conversion change for PROCEDURE EXTERNAL.

Yes, the runtime will do the work. All the info about the PROCEDURE EXTERNAL (library name, parameter types, persistent mode, other clauses) are saved in the name_map.xml and can be used as needed, when it's time to invoke the native procedure.

2. Do we handle PROCEDURE EXTERNAL PERSISTENT? If the runtime is hiding this, then it seems that we would need to remember that the registration would be PERSISTENT in the name map.

Yes, I already save this in name_map.xml. Note that as with IN SUPER procedures, when RUN statement targets a native procedure, this still gets converted to a ControlFlowOps.invoke API call. Later on, the runtime will decide if this is a native procedure call, and if yes, the ControlFlowOps.NativeProcedureCaller will be used to call it.

3, We would still need to implement a RELEASE EXTERNAL as a converted call.

I guess this should convert to a ControlFlowOps.release(<procname>) ?

4. I assume all the parameter conversion can be hidden in the runtime. Since the calling code cannot actually pass the native types, those are just for the runtime to know how to transform the data to match the underlying platform's API.

Correct.

From a conversion perspective, the changes were pretty simple and are almost done (on top of the #1920 files).

The conversion approach: generate a class for each DLL being called, the class should have static methods representing the API.

I'm not sure how you imagined the runtime to work, but I don't think this applies anymore.

For example, doe the library get loaded at that time or is it lazy?

I will need to build tests to check how the library gets loaded and how the PERSISTENT mode works, but I don't think this will affect the converted code.

Good.

After copying the native method info into the name map, how do we handle the PROCEDURE EXTERNAL statement? Do we hide it? Is that already done in the #1920 work?

    The conversion approach: generate a class for each DLL being called, the class should have static methods representing the API.

I'm not sure how you imagined the runtime to work, but I don't think this applies anymore.

Correct. We will write JNI code that calls dlopen (and friends) on Linux and LoadLibrary/GetProcAddress on Windows. We will also have JNI code to actually call the entry points AND to marshall/unmarshall the data to/from the calls. The runtime will already have everything needed to both resolve the RUN native_procedure to the proper JNI calls and to help with the data marshalling/unmarshalling.

There is no longer a need for generating any backing classes.

I will need to build tests to check how the library gets loaded and how the PERSISTENT mode works, but I don't think this will affect the converted code.

We also may need to test if there is any scoping or other reasons to make the each API "procedure" registration explicit. Lazy registration sounds best if possible, but I always get nervous when we have less code than Progress. Usually, when we hide things that they don't hide it comes back to bite us. :)

In regard to RELEASE EXTERNAL, ControlFlowOps.release(<procname>) is fine.

After copying the native method info into the name map, how do we handle the PROCEDURE EXTERNAL statement? Do we hide it? Is that already done in the #1920 work?

Yes, the original procedure definition is not emitted (same as IN SUPER procedure definitions); this is already done part of the #1920 work. Also, warnings will no longer be emitted for PROCEURE EXTERNAL case, as we will add support for this.

We also may need to test if there is any scoping or other reasons to make the each API "procedure" registration explicit. Lazy registration sounds best if possible, but I always get nervous when we have less code than Progress. Usually, when we hide things that they don't hide it comes back to bite us. :)

Assuming we have a custom .so/.dll library, do you know if is there a way to add some kind of loggers at the .so/.dll library level, to check when it is loaded/unloaded ?

Assuming we have a custom .so/.dll library, do you know if is there a way to add some kind of loggers at the .so/.dll library level, to check when it is loaded/unloaded ?

Yes, this is usually quite easy. For example, in WIN32 DLLs can have a "DllMain" function that gets called on process attach/detach and thread attach/detach. That function is optional and would be in your library's source code. You can log things or do most other kinds of initialization or termination processing.

A. misc findings for PROCEDURE EXTERNAL case:
- the name used to define this kind of procedure is case-sensitive(to be able to map it to an exported entry point by the dll), but is case-insensitive when used in RUN statements
- the MEMPTR type can be set for the parameters (INPUT,OUTPUT,INPUT-OUTPUT,RETURN).
- although GET-SIGNATURE reports RETURN parameters as OUTPUT, I think we might need to log the mode as RETURN in name_map.xml (to be able to set the returned value correctly), and change GET-SIGNATURE to return OUTPUT for them.
- if ORDINAL n clause is present, then the exported entry at that index will be invoked (the match will not be done by name, the exported entry point at the given index will invoked, regardless of the name).
- no parameter validation seems to be done. if the number of parameters (or their types) do not match, then 4GL sees that "stack is corrupt" and abend the progam.

B. when a RUN statement targets a PROCEDURE EXTERNAL:
0. if the library can't be loaded, it will fail:

Could not load procedure DLL procedure <pname>. (3258)

1. if the definition has the PERSISTENT clause, then:
- on the first call, will keep the library in memory if the entry point is found (what if entry point generates errors?).
- on the first call, will load and unload the library, if the entry point is not found
- once loaded, on subsequent calls, sees the library already loaded and will not load it again

2. if the definition doesn't have the PERSISTENT clause will:
- load the library, if it wasn't already loaded by a prior call for a PROCEDURE EXTERNAL ... PERSISTENT defined for the same library.
- regardless of the entry point was found or not, it will unload the library if it wasn't already loaded prior to this call

This suggests that the library is saved in memory on the first call of a valid exported entry point, and the procedure definition has the PERSISTENT clause.

C. RELEASE EXTERNAL <library>
- once this is called, the library is unloaded from memory and rules at section B. apply, as if it was never loaded.
- if the library doesn't exist, no error is reported
- don't know what happens if the library doesn't want unload successfully

D. CDECL/PASCAL/STDCALL
- CDECL is for C calling convention when accessing the routine
- PASCAL is for backward compatibility
- STDCALL is for Windows calling convention when accessing the routine
Need to investigate how these might affect the library loading, entry point resolution and routine invocation.

1. if the definition has the PERSISTENT clause, then:
- on the first call, will keep the library in memory if the entry point is found (what if entry point generates errors?).

Interesting. The PROCEDURE EXTERNAL doesn't load the library, no matter if it has PERSISTENT or not?

2. if the definition doesn't have the PERSISTENT clause will:
- load the library, if it wasn't already loaded by a prior call for a PROCEDURE EXTERNAL ... PERSISTENT defined for the same library.
- regardless of the entry point was found or not, it will unload the library if it wasn't already loaded prior to this call

Are you saying that the DLL is loaded and unloaded for every call (on a DLL that has never been loaded PERSISTENT)? That is a horrible design. There would be huge overhead in doing this in a loop (for example). The documentation suggests that the library is loaded for the scope of the procedure and then unloaded when the procedure exits unless it was loaded PERSISTENT.

C. RELEASE EXTERNAL <library>
- once this is called, the library is unloaded from memory and rules at section B. apply, as if it was never loaded.

What happens when you call an API (using RUN on a PROCEDURE EXTERNAL) after the RELEASE EXTERNAL? Doe the API call work? Is there an error?

D. CDECL/PASCAL/STDCALL
- CDECL is for C calling convention when accessing the routine
- PASCAL is for backward compatibility
- STDCALL is for Windows calling convention when accessing the routine
Need to investigate how these might affect the library loading, entry point resolution and routine invocation.

The calling convention should not affect library loading or entry point resolution at all. However, it is absolutely essential to the entry point invocation.

Don't worry about investigating this now. It is purely a runtime issue. This is part of the marshalling/unmarshalling of parameters and return values that will have to occur. We will be hiding much of it in the JNI layer and in fact must do much of it there.

Each calling convention is a "contract" that defines how parameters are passed and data is returned. But it also defines things like which registers are used or preserved, how much of the data is passed via the stack, the order of the parameters on the stack, the alignment of parameters on the stack... APIs are compiled with a calling convention defined. The WIN32 API is STDCALL. Many C libraries are CDECL. Older libraries can be PASCAL (and it should be pretty rare now). There are many other calling conventions but the 4GL only supports these 3.

Luckily, all 3 are primarily stack based. But they are still quite different. To give you can idea of the differences:

CDECL: pushes onto the stack right to left (reverse order of how they are written in the code) and caller cleans the stack; non-floating point return values come back in EAX; the caller must save EAX, ECX and EDX because these will not be preserved across the call

STDCALL: similar to CDECL except the callee cleans the stack

PASCAL: pushes left to right and the callee cleans the stack

Normally, DLL entry points are called from code that was compiled against the specific API in question. This means that the compiler has a definition of the API (its signature AND its calling convention) and the compiler then hard codes the CPU instructions before and after the call to properly implement the invocation.

Even in cases where you are using "runtime linking" which is what dlopen/LoadLibrary are providing, normally the compiler can help you out. This is done by defining the API call as a function pointer (where all the parameters and return value are properly typed). So the type of the function pointer is "hard coded" into the program (thus it is known at compile time) BUT the value of the pointer is obtained at runtime. But this is sufficient for the compiler to do the "heavy lifting" for the programmer. It takes care of everything just like a load-time linked or statically linked function call.

Unfortunately, this is not possible for us. Our JNI code will not be compiled with all the signatures of the APIs known, so the compiler cannot help us here. This is going to require some assembly code to do the job right...

It isn't a problem. In fact, it will be some fun to work on. But that is for later. It is all runtime stuff and we just need to get the conversion done for now.

The PROCEDURE EXTERNAL doesn't load the library, no matter if it has PERSISTENT or not?

The fact that there is a PROCEDURE EXTERNAL defined in current procedure, will not load the library. That is done only on first run.

Are you saying that the DLL is loaded and unloaded for every call (on a DLL that has never been loaded PERSISTENT)?

I've used DllMain to catch the process/thread attache/detach and this is what how it seems to work (on Windows anyway). Only when the target is defined PROCEDURE EXTERNAL PERSISTENT, the library remains loaded until program end. Still need to test the case where program P1 loads the library, invokes program P2 which invokes a PROCEDURE EXTERNAL in the same library.

What happens when you call an API (using RUN on a PROCEDURE EXTERNAL) after the RELEASE EXTERNAL? Doe the API call work? Is there an error?

I'll need to test this.

About the conversion changes: the changes are in files changed by 1920/1921 work, and I want to submit them after that goes to repository.

This looks good to me. If that is everything that is needed, then you can go ahead and convert/test this.

Actually, I think the runtime part is pretty safe. So if it passes conversion testing, no runtime regression testing is needed on this one.

Passed conversion regression testing, applied to lightning and committed to bzr revision 10156.

Constantin: can you define the native API you will need in order to finish the ControlFlowOps support for native procedures?

I plan to do the implementation. I can estimate the effort better if I have an API to look at. You will implement the portion in Java that calls the API and I will implement the API (mostly as native methods) and the JNI backing for it. The big effort will be in the parameter/return value marshalling/unmarshalling.

I'd also like to know how long you would expect to need to finish your portion (given that the API will be done separately).

• determine how the argument validation is done when the determined procedure is EXTERNAL (what error messages are shown, is ERROR condition raised, when is the argument validation done - at procedure resolution or at native routine invocation or both, etc)
• determine if/how extents are passed
• in the docs for DEFINE PARAMETER there is a table mapping the native types to "ABL DLL types", but there is no mapping of the "ABL DDL types" to "ABL data types"; I think we need to determine this too.
• ControlFlowOps.NativeProcedureCaller.invoke needs to invoke an API to which it needs to pass, beside the arguments, the dll name, the procedure name and the 4GL program name where PROCEDURE EXTERNAL is defined. We can add a NativeInvoker class with a public static void invoke(String dllName, String pname, String iename, Object... args) API, where pname is the 4GL program name and iename is the internal entry name. At this time the ControlFlowOps.NativeProcedureCaller has no knowledge about the procedure which needs to be called, but this can be easily added. Note that currently the javadoc for pname parameter for the NativeProcedureCaller c'tor is incorrect, pname is the legacy 4GL program name and not the internal entry name.
• something else we need to determine is if any native-related data is maintained in the external program context where the library was loaded (here I refer to something like - does the library get unloaded when the handle for the external program context gets deleted? and other similar problems). If yes, then the NativeInvoker.invoke API needs a reference to the procedure handle targeted by the RUN call. The p2jlib.so native code would not receive the instance reference, but a hashcode of the procedure instance. But, I want to think about this after we determine how the loading/unloading gets done... I guess you will investigate this?
• The NativeInvoker.invoke needs to obtain the mode for each parameter plus their declared types (please check if the HANDLE TO clause is used with the DEFINE PARAMETER statement). We need to add an additional marker for the DEFINE RETURN PARAMETER case, as the GET-SIGNATURE reports this as OUTPUT (and the RETURN mode gets lost during conversion). The idea is the RETURN parameter will hold the value returned by the native call. Also, at this time we need to determine the ORDINAL value (if present) and the CDECL/PASCAL/STDCALL modes (if present); the SourceNameMapper needs additional APIs for this.
• Once the parameters are determined, we need to call an entry point in libp2j.so, via the public static native int NativeInvoker.invokeRoutine(String dllName, String routine, int ordinal, String callMode, String modes, String[] signature, BaseDataType[] args) API, where:
  - see next point for what this API returns
  - ordinal is the ORDINAL n clause; 1 if ORDINAL n clause is not present.
  callMode is one of the CDECL/PASCAL/STDCALL strings
  - modes is the string representation of the modes for each passed argument, at the RUN call; not sure if we need to pass the declared mode (at the PROCEDURE EXTERNAL definition) for each parameter or not. If no RETURN parameter is defined, the char on position zero will be a space, else "R".
  - signature is an array with the declared type for each parameter; element 0 is the RETURN parameter, if exists; else, null
  - args is an array with the passed parameters; element 0 is the RETURN parameter, if exists; else, null
  At the time of this call, we ensure all arguments are BDT and the declared type of each argument is determined. The native code will take care of marshalling/unmarshalling. Note that if extents can be passed, then args will need to be defined as Object[] args.
• NativeInvoker.invokeRoutine will return 0 if the call was OK or an invocation failure code, depending on:
  - if the library could be loaded or
  - if an entry point was found or not
  - if the entry point has a different signature than the passed declared signature

The complex part in the Java code is the argument validation part. The java code (excluding argument validation) can be done in ~5-6 hours of work. For the argument validation, I want to reserve ~2 days of work (~16-20 hours).

I was planning to build the part of the solution from the NativeInvoker.invokeRoutine() on down to the OS. It is not clear (to me) what the NativeInvoker.invoke() code is meant to do or why it needs certain things (e.g. like the names of the 4GL program name or internal entry name).

Please assume that you would be writing that portion. In such a case, what additional time is needed?

I think that the NativeInvoker.invokeRoutine() would also need to know if this was being RUN PERSISTENT or not.

In terms of the approach to the NativeInvoker.invokeRoutine() code, here is the basic plan:

1. Research to determine all the "ABL to/from DLL" data type transformations/equivalents. This would include examining the EXTENT parameter issue.

2. Any additional research to determine how loading/unloading works, as well as any dependencies on procedure handles. Please note that I'm not clear on why the native code would need access to a hashcode of the procedure instance.

3. The Java portion of the NativeInvoker.invokeRoutine() would be implemented. We will maximize the amount of preparation and processing that is done there. For example, we will likely convert the mode list into an int[] with one element per parameter and with specific codes to describe the corresponding element of the parameter array. This will be a format that is much easier to process in the native C code. Another example of a likely task for this layer of the solution is to separate the return "parameter" from the parameter array. None of the calling conventions that we need to support naturally will handle the concept of a return parameter. Instead, the return value for a function call is returned in EAX (on x86 systems). We will probably pass the wrapper instance in which the return value should be assigned, as an object reference to the JNI call. The value will then be assigned back to the wrapper instance in the native code. The parameter array that is passed would only contain the values passed on the stack and considered "real" parameters. We would also translate the calling convention into an int code instead of passing a string.

4. The native C code would provide a function that is called by NativeInvoker.invokeRoutine() to implement the majority of the native processing for each API call. It must provide the following:

• load the library
• lookup the function address based on ordinal or name
• translate the parameter array into an array of in-memory native C data types that are properly structured as the full parameter list of the function call
• use an assembly language function dispatcher to actually call the function
• handle the assignment of the return value, if needed
• determine the return code from the operation and return it back
• of course, all suitable error checking and handling will be provided

5. For each CPU architecture (Intel x86 and Solaris at a minimum; we will have to see if there is any need to differentiate between 32-bit and 64-bit x86), there will be a .c source file with a set of dispatching functions. There will be 1 dispatching function for each possible native data type that can be returned. Each of these functions will quickly call into common code. Inside each dispatching function there will be inline assembly code that does the following (based on calling convention):

• pushes the parameters onto the stack in the appropriate order (right to left for CDECL and STDCALL; left to right for PASCAL) and with the proper machine word size
• saves off any registers that must be preserved across the call (which are known to not be preserved in that convention)
• invokes the function via pointer (this is synchronous of course)
• cleans the stack if the calling convention requires the caller to clean (should be CDECL only)
• handles any return value differences as needed (generally most values return in EAX, but there may be some special cases like floating point), basically the registers need to be setup properly to match the return value processing for the given data type (this part won't be common code, but will be directly in the specific version of the dispatching function)

6. All of this C code will be compiled into libp2j.so and the build process will need to be updated to handle the new files and the CPU architecture differences. Note that so long as mingw (our Windows compiler that is a variant of gcc) can handle inline assembly, we at least won't have to use an assembler directly, nor will we have to change the makefile as much.

7. We will have to handle the RUN PERSISTENT case by maintaining the library "handle" (in OS terms) and the specific function pointer for the API. We need to determine various things about where it is stored (Java or native; if in Java, what code), how it is stored (the data structure), how it is initialized and how/when it is destroyed. I do suspect that the storage should be handled on the Java side. A more granular version of the native invocation mechanism may be needed for the RUN PERSISTENT case, such that the loading (both the library if not already loaded and the function address), function call and unloading are all separated and called individually as needed from the Java code. This is probably the best way.

Note that we will probably avoid the Solaris implementation at this time and defer that work to #1896.

LE: I expect the portions described here will take up to 80 hours to complete.

Greg Shah wrote:

I was planning to build the part of the solution from the NativeInvoker.invokeRoutine() on down to the OS.

Looks like I skipped the "native" modifier for the NativeInvoker.invokeRoutine() signature. Idea is this is a native method, not a Java method. See below how its signature can be changed.

It is not clear (to me) what the NativeInvoker.invoke() code is meant to do or why it needs certain things (e.g. like the names of the 4GL program name or internal entry name).

I want to let the ControlFlowOps.NativeProcedureCaller.invoke just invoke the NativeInvoker.invoke, which will do all the work. The 4GL program name and iename are needed to determine the ORDINAL clause, each parameter's mode, type, etc from the internal entrie's definition.

Please assume that you would be writing that portion.

See above, I want NativeInvoker.invokeRoutine() to be a native method.

In such a case, what additional time is needed?

Some additional time is needed to determine the string-to-int mappings for the parameter modes/types/etc. If possible, I don't want to duplicate these constants in both Java and C code... but, beside adding some public static native int NativeInvoker.encode{Mode|Type|etc}(String) APIs which will encode this, I can't think of any other way. For the returned error codes, maybe it will be better to return a constant from NativeInvoker or some other interface, via JNI code.

I think that the NativeInvoker.invokeRoutine() would also need to know if this was being RUN PERSISTENT or not.

• does a library loaded via a PROCEDURE PERSISTENT call gets unloaded when the persistent 4GL procedure gets deleted? (assuming the PROCEDURE PERSISTENTprocedure is executed via a RUN proc IN handle call)
• if a library is loaded by calling a PROCEDURE PERSISTENT from a persistent 4GL procedure, using a RUN proc IN handle call, will another call of a PROCEDURE PERSISTENT in the same library and using a RUN proc IN handle2 (where handle2 is for another persistent 4GL procedure) load the library again or will use the first loaded instance?

In terms of the approach to the NativeInvoker.invokeRoutine() code, here is the basic plan:

2. Any additional research to determine how loading/unloading works, as well as any dependencies on procedure handles. Please note that I'm not clear on why the native code would need access to a hashcode of the procedure instance.

In case the library gets somehow "hooked" to the 4GL program instance which initially loaded it (and gets unloaded when the procedure instance gets deleted), then we will need a way of mapping the loaded library to this procedure instance. Anyway, these are just suppositions at this time.

3. The Java portion of the NativeInvoker.invokeRoutine() would be implemented. We will maximize the amount of preparation and processing that is done there. For example, we will likely convert the mode list into an int[] with one element per parameter and with specific codes to describe the corresponding element of the parameter array. This will be a format that is much easier to process in the native C code. Another example of a likely task for this layer of the solution is to separate the return "parameter" from the parameter array. None of the calling conventions that we need to support naturally will handle the concept of a return parameter. Instead, the return value for a function call is returned in EAX (on x86 systems). We will probably pass the wrapper instance in which the return value should be assigned, as an object reference to the JNI call. The value will then be assigned back to the wrapper instance in the native code. The parameter array that is passed would only contain the values passed on the stack and considered "real" parameters. We would also translate the calling convention into an int code instead of passing a string.

OK, as I want this to be a native call, we can change its signature. What about this (note that I'm open to changing this in any other way which can ease work in the C code):

public static native int NativeInvoker.invokeRoutine(String dllName, String routine, int persistent, int ordinal, int callMode, int[] modes, int[] signature, BaseDataType[] args, BaseDataType ret)

7. We will have to handle the RUN PERSISTENT case by maintaining the library "handle" (in OS terms) and the specific function pointer for the API. We need to determine various things about where it is stored (Java or native; if in Java, what code), how it is stored (the data structure), how it is initialized and how/when it is destroyed. I do suspect that the storage should be handled on the Java side.

If storage is managed by Java side, then I guess this will be done by library name, and this will include the library's full path, and not relative path. More, we will need to take care of case (in)sensitive paths/names. Again, please confirm that you refer to PROCEDURE PERSISTENT.

A more granular version of the native invocation mechanism may be needed for the RUN PERSISTENT case, such that the loading (both the library if not already loaded and the function address), function call and unloading are all separated and called individually as needed from the Java code. This is probably the best way.

Note sure what you are thinking about this: what usage would have the Java code about the function address?

• for the PROCEDURE PERSISTENT case, we will have a public static native int NativeInvoker.loadLibrary(String dllName) API. It returns 0 on success, else an error code describing the failure. This API can assume the library exists on the system.
• we need to determine how the library gets loaded/unloaded when the PROCEDURE definition does not have a PERSISTENT clause. Will you further analyze this? In note 12 my initial findings showed that the library does get unloaded if it was explicitly loaded for this call. More, this loading can be done using an algorithm like this, from the Java code:
  - determine if library is already loaded
  - if not, explicitly load the library for this call
  - invoke the native routine
  - unload the library, if explicitly loaded
• RELEASE EXTERNAL needs a public static native int NativeInvoker.unloadLibrary(String dllName), with returned values similar to loadLibrary

As always, the hard part is not coding the behavior, is determining the correct behavior. Let me know if you want me to help with some other analysis, beside parameter validation.

Looks like I skipped the "native" modifier for the NativeInvoker.invokeRoutine() signature. Idea is this is a native method, not a Java method.

This is no problem, so long as NativeInvoker.invoke() does the Java-side preparations as documented above.

Please assume that you would be writing that portion.

I was referring to NativeInvoker.invoke(). I'd like you to take that part. I will handle NativeInvoker.invokeRoutine() and all other native portions.

In terms of the approach to the NativeInvoker.invokeRoutine() code, here is the basic plan:

2. Any additional research to determine how loading/unloading works, as well as any dependencies on procedure handles. Please note that I'm not clear on why the native code would need access to a hashcode of the procedure instance.

In case the library gets somehow "hooked" to the 4GL program instance which initially loaded it (and gets unloaded when the procedure instance gets deleted), then we will need a way of mapping the loaded library to this procedure instance. Anyway, these are just suppositions at this time.

Since the library management will be done on the Java side, I don't think the native code will need the procedure handle.

public static native int NativeInvoker.invokeRoutine(String dllName, String routine, int persistent, int ordinal, int callMode, int[] modes, int[] signature, BaseDataType[] args, BaseDataType ret)

I think this is pretty close. What about the following additional changes:

• change dllName to library since this is a cross-platform solution
• change callMode to callConv
• persistent can be boolean
• I don't think we need the signature array in the native code (what does it tell us that we can't already tell from the types in the args array?)

Again, please confirm that you refer to PROCEDURE PERSISTENT.

Yes, sorry for the confusion.

A more granular version of the native invocation mechanism may be needed for the RUN PERSISTENT case, such that the loading (both the library if not already loaded and the function address), function call and unloading are all separated and called individually as needed from the Java code. This is probably the best way.

Note sure what you are thinking about this: what usage would have the Java code about the function address?

I am referencing the idea that there are 2 possible calling sequences to the native code:

For procedure persistent invocations we will use the "more granular" set of methods:

1. load the library and the routine address
2. call the invocation 1 or more times
3. unload the library

For non-persistent invocations, the invokeRoutine() will do all of it in 1 call. The idea here is that JNI calls are relativelyu costly. We don't want to make 3 native calls when 1 will do.

For the library and function address loading/unloading process, we have some things to determine before we can know exactly how it will work. Generally speaking, a dynamically loaded library can only exist as 1 instance in a given process. So, multiple PROCEDURE EXTERNAL instances will all use the same library instance. That is an OS-level implementation choice and cannot be overridden by Progress. But the unloading process can be influenced by Progress, because they can keep the library around an arbitrary amount of time, beyond when the last usage occurs or not as they decide. The OS doesn't reference count the number of times function addresses have been queried or used, so those usage patterns have no bearing on unloading a library. The only requirement on the part of the OS is that no code can be currently executing in that DLL. Even though some OS implementation may even ignore this fact when unloading, the results would be bad if it were to happen.

What is the total time you estimate needing for your parts of this task?

I was referring to NativeInvoker.invoke(). I'd like you to take that part. I will handle NativeInvoker.invokeRoutine() and all other native portions.

OK

I think this is pretty close. What about the following additional changes:

• change dllName to library since this is a cross-platform solution
• change callMode to callConv
• persistent can be boolean
• I don't think we need the signature array in the native code (what does it tell us that we can't already tell from the types in the args array?)

How will the native code know to convert a value from "ABL data type" to "ABL DLL type" (better said, if signature is not sent, how will the native code know what is the "ABL DLL type" for a certain value)? More, what if a library has overloads of a certain routine? Wouldn't the real signature (and not the values passed as arguments) be needed to disambiguate between multiple routines with the same name?
And another one: I wonder how _POLY cases behave in this case... will a _POLY still get automatically converted to the expected type, if compatible?
And finally, 4GL allows this:

procedure proc1.
  def input param i as int.
  def output param j as int.
  message "made it".
  j = 11.
end.

def var ch as char.
run proc1(input "0", output ch). /* proc1 is executed at this call */
message ch. /* here, ch is "11" */

The signature can be:

public static native int NativeInvoker.invokeRoutine(String library, String routine, boolean persistent, int ordinal, int callconv, int[] modes, int[] signature, BaseDataType[] args, BaseDataType ret)

I am referencing the idea that there are 2 possible calling sequences to the native code:

OK, we need to pass this info to invokeRoutine(), to know if it needs to load/unload the library. We can use the persistent parameter, which on false means the library must be loaded/unloaded by this call. The Java code will always ensure the library is loaded before a a call of a PROCEDURE PERSISTENT, so it will pass the persistent parameter to true; this will also be true if the procedure doesn't have the PERSISTENT clause, but a prior PROCEDURE PERSISTENT call has loaded the library.

What is the total time you estimate needing for your parts of this task?

At this time, I expect 30 hours of work, unless some new behavior is found which needs to be worked in Java code (sorry, but I can not eliminate this possibility, as during initial analysis we can only foresee this much).

How will the native code know to convert a value from "ABL data type" to "ABL DLL type" (better said, if signature is not sent, how will the native code know what is the "ABL DLL type" for a certain value)?

OK, I see your point. If we pass the args as BDT, then we would need more information about the signature. The advantage to doing it that way is it make it much easier to implement input-output or output parameters. BUT, the disadvantage is that we must encode much more information AND we must make the native layer much more complex.

Ultimately, only the following types (unless we find otherwise in testcases) need to be supported:

BYTE
SHORT
UNSIGNED-SHORT
LONG
UNSIGNED-LONG
INT64
FLOAT
DOUBLE
HANDLE TO BYTE
HANDLE TO SHORT
HANDLE TO UNSIGNED-SHORT
HANDLE TO LONG
HANDLE TO UNSIGNED-LONG
HANDLE TO INT64
HANDLE TO FLOAT
HANDLE TO DOUBLE
CHARACTER
LONGCHAR
MEMPTR

The HANDLE TO form just means that we pass the primitive type via pointer, which allows the library to modify the content (how C does INPUT-OUTPUT and OUTPUT parameters).

CHARACTER, LONGCHAR and MEMPTR are always passed by pointer.

Pointers will be either 4 or 8 bytes in size, depending on the machine word size.

All other data types (that are not pointers: BYTE, SHORT, UNSIGNED-SHORT, LONG, UNSIGNED-LONG, INT64, FLOAT, DOUBLE) are a type understood by the CPU itself and they range in size between 1 and 8 bytes.

From the perspective of the native code, we need the data to be in one of the above listed formats. That means that we must transform whatever BDT is given into the proper type. I prefer that to be done in the Java code. The HANDLE TO case will have to be implemented in the native code as will the CHARACTER case.

But one option is to define a set of simple wrapper classes for these types. Each one would contain:

• the value itself, in a way that can be modified
• the BDT reference to which this is linked (we proably need this to be an Accessor to handle FieldReference instances)
• a flag to indicate the HANDLE TO case
• a flag to indicate if we need to assign any modified value back (INPUT-OUTPUT or OUTPUT modes)
• constructors to accept a reference to the "compatible" BDT types or to an Accessor
• a method to obtain the data in the correct native type (this would convert the linked BDT data on the fly)
• a method to assign the correct native type back (into the Accessor) if needed

The data itself can be stored in the base class (BaseNativeType). The data type specific processing can be in each child class. The arguments would then be specified as BaseNativeType[] and we would have everything needed to dispatch at the native level. I think this is cleaner and delegates more work into Java, while preserving the ability to assign back.

The return value reference will also need to be BaseNativeType, but we would only ever assign back to it.

More, what if a library has overloads of a certain routine? Wouldn't the real signature (and not the values passed as arguments) be needed to disambiguate between multiple routines with the same name?

Luckily, this is not an issue. Shared library/DLL support does not provide overloading capabilities directly. That is typically implemented as a language-level feature. So in Windows, you call GetProcAddress(handleOfPreviouslyLoadedLibrary, routineNameToLookup). You don't ever pass a signature. Interestingly, the OS itself does not know the signature of the function! When the library is loaded, the exports are "registered" as addresses mapped to specific names and/or ordinals. Of course, the addresses must be resolved at load time because they will depend on literally where in memory the library was loaded. We are literally obtaining the memory address for the compiled code of the function itself.

The way that overloading is done is that a language will either use ordinals (rare) or it will "mangle" the function name (common, e.g. C++ does it this way). Mangling basically encodes signature information into the generated function name at compile time. This makes using runtime-based dynamic linking (e.g. directly looking up function addresses) much more difficult. But most C++ code would never use this approach anyway, as that stuff would be calling objects created locally in process and would be able to easily link at compile time (even with code that resides in a library), thus hiding all of the mangling complexity.

And another one: I wonder how _POLY cases behave in this case... will a _POLY still get automatically converted to the expected type, if compatible?

Good question. I imagine so, but we will certainly have to check.

I assume that the _POLY case cannot be used as INPUT-OUTPUT or OUTPUT parameters, since it is a temporary reference and not an lvalue.

The idea of all this is that the values' type don't always fully follow the procedure's signature, at a RUN statement call.

Insane. Definitely a concerning possibility. We should test these cases and handle them inside the BNT (BaseNativeType) classes.

At this time, I expect 30 hours of work, unless some new behavior is found which needs to be worked in Java code (sorry, but I can not eliminate this possibility, as during initial analysis we can only foresee this much).

I am going to specify 40 hours for that part, to account for some crazy/unexpected behavior. As you note, the issue is finding and understanding the behavior, the actual coding will not take long.

Combined with the 80 hours I expect to need on the native side, I will update the hours for #1959 to 120 (from 104).

Good work. As usual, your involvement has helped to make the solution better.

About the parameter validation: as the native side has no idea of the routine's signature (thanks for the details, it really helped to get a picture on how things get invoked at OS level), than it can not validate them at all (if incompatible value is sent, then I guess the stack/registers? will be corrupted and routine invocation will fail at some point). So, I think all the validation which 4GL performs on these parameters is done using the signature for the 4GL PROCEDURE definition. This is good news.

I assume that the _POLY case cannot be used as INPUT-OUTPUT or OUTPUT parameters, since it is a temporary reference and not an lvalue.

Actually, there is the :: operator which is _POLY... and might be able to be used with OUTPUT/INPUT-OUTPUT. Testing is needed to confirm.

In thinking about this, I have come to the conclusion that the only correct way to implement this is on the client-side. Initially, I was thinking we could implement this such that it could be used in either the client or the server, but upon reflection this must be a client-side exported service like process launching or file system access.

Shared libraries/DLLs have many interesting behaviors. Such modules will often contain and maintain state. On most systems, such libraries can declare this state as "global" data (shared across all processes on the system) or "instance" data (unique to a given process). Because of the ability for DLLs to have data that is per-process, it is imperative that we implement native library support in the client side, where it will naturally mimic the same separation of processes that occurs in the 4GL. There is no other way to duplicate this behavior, since it is implemented at the OS level and the libraries being called are all 3rd party code that (we must assume) cannot be modified.

Loading/Unloading

1. No loading of a library occurs until a RUN statement is executed which matches a PROCEDURE EXTERNAL definition.
2. Any RUN statement that is executed which matches a PROCEDURE EXTERNAL definition will attempt to load the referenced library. If the library can be found and the OS can load it, then the OS will return a "handle" to the loaded library.
3. That handle is not a 4GL handle, but just an OS-specific opaque ID that is used to reference the library.
4. Any RUN statement that is executed which matches a PROCEDURE EXTERNAL definition which has the PERSISTENT option enabled, will cause the loaded library to remain loaded after the function call is complete.
5. If the referenced PROCEDURE EXTERNAL is not defined as PERSISTENT AND that library was not previously referenced via a PROCEDURE EXTERNAL which was defined as PERSISTENT, then the library (if it can be found and if it can be loaded) will be loaded before the function call, then unloaded afterward.
6. On Linux, a RELEASE EXTERNAL statement will always be accepted without error with one exception: passing unknown value for the library name causes a silent error (even when NO-ERROR is missing!) to be raised. There is no error text or error number shown on the terminal OR recorded in error-status in NO-ERROR more.
7. Although RELEASE EXTERNAL will accept any input (including complete trash), on Linux any library loaded PERSISTENT is never unloaded until the 4GL process ends. It is expected that on Windows, the library will actually unload.
8. There is no reference counting that is pinning the library into memory on Linux. Even when RELEASE EXTERNAL was run 100 times in a loop (far more times than there had been calls to the library), the library still did not unload.
9. On Linux, if the library name in the PROCEDURE EXTERNAL contains no pathing, then the OS will use a library search algorithm to find the match. The Linux OS (and Progress 4GL) does not add a .so or a lib prefix to the library name, rather it searches for exactly the name you specify. So libtestapi.so cannot be found using "testapi", "libtestapi" or "testapi.so". Likewise, on Linux a name with mismatching case like "libtESapi.so" will not work to load "libtestapi.so".
10. On Linux, a relative path like "library_calls/testapi/libtestapi.so" will work if the library actually exists in this relative to the process' current working directory. Absolute paths like "/usr/lib/libtestapi.so" also work.
11. I saw no evidence that Progress did anything to modify the library name that was passed, I believe that the standard OS facilities were used.

I have now added many more tests. In addition, I have gotten all tests running in 32-bit OE 10.2B06 in both Windows 32-bit environments (a real Windows 32-bit installation and the WOW environment in 64-bit Windows Server 2008). I am still waiting for a working 64-bit Progress 11.3.1 environment in order to test the final scenario.

The good news so far on the Windows 32-bit support is that it works identically in both 32-bit environments, except in the WOW environment it is more likely to have some trash in the upper words of registers/stack slots which can appear as unexpected values when there is a mismatch in parameter types between what Progress is told versus what the real function implements.

Loading/Unloading Additional Findings

1. On Windows, you can load using no path, relative path or an absolute path (starts with a drive letter and colon). In each of these scenarios, the case of the library name string does not have to match the actual case in the file system. This is expected because in most circumstances, the Windows file system is case-preserving but matches are case-insensitive.
2. On Windows, it is also possible to leave the library extension (.dll) off of the specified library name during loading and the operating system itself (the LoadLibrary API) will add it (actually, it will add ".DLL" if you have no extension). I don't think the 4GL does this.
3. On Linux, if you try to load with an absolute or relative path name included which is only incorrect because it uses the wrong path separators (backslashes instead of slashes), it will fail.
4. On Windows, if you try to load with an absolute or relative path name included which is only incorrect because it uses the wrong path separators (slashes instead of backslashes), it will SUCCEED! The LoadLibrary call does not do this, so this must be special processing done by the 4GL.
5. This means that the number of possible load failure cases on Windows is reduced, since case-matching and the extension are not failure conditions. There is no leading "lib" prefix like on Linux. A non-pathed library name must not exist in the PATH or current directory by the exact name specified OR by that name with a ".DLL" added. A pathed library name must not exist in that relative or absolute location by that name or by that name with a ".DLL" added.
6. On Windows, the RELEASE EXTERNAL always silently executes (except for unknown value which causes a error with no message) no matter the validity of the library name provided, just like in Linux.
7. On Windows, the RELEASE EXTERNAL actually DOES WORK (not like in Linux), however it is subject to some limitations. The key is that if you specify the exact same name as was used to load the library persistently, then the first time that RELEASE EXTERNAL is called after the successful LoadLibrary, the corresponding UnloadLibrary call will work (causing the library to really be unloaded from the process).
8. Here are some subtle points about how unloading works:
   • LoadLibrary is only ever called once for a given library name input string. This means that the operating system reference count USUALLY does not go over 1. If the reference count does go above 1 (see below), then you must have as many UnloadLibrary calls as LoadLibrary calls before the library is actually unloaded.
   • While normal 4GL string processing ignores trailing whitespace and case, the LoadLibrary tracking does not have these properties. Any whitespace or case difference is considered a different string. Likewise, although the 4GL seems to convert backslashes to slashes when it passes the library name string to LoadLibrary, it does not track the loaded library with the modified string (see unloading below). In addition, loading the same library with an without the ".dll" extension is considered to be loading 2 different libraries for tracking purposes. This is the cause of potential reference counting problems (see below).
   • Unloading the library works strangely from a 4GL perspective because two library name input strings that differ from the string used for loading, IN ANY WAY, will be treated as different libraries. That means that the same name that differs by case, whitespace (trailing, leading or in the middle), path separator characters or by having the optional .dll extension, will be treated as a different library. The UnloadLibrary call is only called once for each matching LoadLibrary call, so the silent failure that occurs when a slightly different name is passed will cause the library to not be unloaded when one might normally think that it would.
   • To make it clear, the following pairs are NOT EQUIVALENT FOR UNLOADING even though they ARE EQUIVALENT FOR LOADING: ("testapi" vs "testapi.dll", "teStapi.dll" vs "tEstapi.dLl", "relative/path/testapi.dll" vs "relative~\path~\testapi.dll").
   • Once an unload has occurred for a given name, RELEASE EXTERNAL of the same name will not cause another attempt to unload at the OS level unless there has been a successful persistent load request for that name since the unload occurred. We know this because of tests written to try to create reference counting issues.
9. It is possible to load "trick" the 4GL into calling LoadLibrary more than once for the same library. Its tracking of loaded libraries is not very smart. On case were the problem happens is when the name used is automatically augmented by Windows (testapi will load the same thing as testapi.dll). But the 4GL treats these are 2 different things, so it will call LoadLibrary, twice for what is the same DLL, causing Windows to increment the reference count. This leaves the DLL in memory until some 4GL code calls RELEASE EXTERNAL for BOTH names (testapi and testapi.dll). In other words, if we call a PROCEDURE EXTERNAL PERSISTENT for both "testapi" and "testapi.dll" (it can be once each or multiple times each, it doesn't matter), then the 4GL will call LoadLibrary twice and Windows will load the testapi.dll once and then the second time it will return the same HMODULE (DLL module handle) but it will have incremented the reference count to 2. You can call release external on "testapi.dll" 100 times (of course, Progress really only calls UnloadLibrary for the first such call) and it would not unload the library until a release external is called for "testapi".
10. Since the 4GL doesn't seem to track the actual path of the module that is loaded AND since it seems to be quite "dumb" about the text matching for module names (and how release external uses them), it is expected that this same reference counting problem would occur if a relative or absolute name was used and it happened to load the same DLL that was loaded via some other name. Likewise, this probably occurs with case mismatches too.
11. Interestingly, Progress could actually use the WIN32 GetModuleFileName() on the module handle to get the real path and do something more intelligent with its tracking, but it doesn't.
12. In terms of error processing, all input parameter checks are executed first (and they will fail before a load ever is attempted).
13. Error processing on load failures in Windows: once a load is attempted, any failure causes error 3258:
    • Could not load DLL procedure <library_name_used_in_the_procedure_external_def>. (3258).
14. Error processing on load failures in Linux: once a load is attempted, any failure causes 3 errors to be raised in this order:
    • Could not open Dynamic Library: <library_name_used_in_the_procedure_external_def> (8013)
    • DLL Error : <library_name_used_in_the_procedure_external_def>: cannot open shared object file: No such file or directory (8014)
    • Could not load DLL procedure <library_name_used_in_the_procedure_external_def>. (3258).

Function Pointer Lookup

Once the library is successfully loaded, a pointer to the function that is to be called must be obtained. Remember, this is a form of runtime linking where everything is dynamic and done at the time of use. For non-persistent cases where the library is not already loaded, the process is LOAD, FIND POINTER, CALL POINTER, UNLOAD. For persistent cases where the library is not already loaded, the process is LOAD, FIND POINTER, CALL POINTER. For persistent and non-persistent cases where the library is already loaded, the process is FIND POINTER, CALL POINTER.

There is no evidence of whether Progress caches the pointer associated with an external function. But there is no reason not to do so. In the "already loaded" cases above, this would eliminate the FIND POINTER step. When a library is loaded into the process' address space, any relocatable code and references are fixed up and thereafter the exported entry points will not move their location. For this reason, one can rely upon the pointer to a specific function to remain the same so long as the library is not unloaded. Of course, in the non-persistent cases where the library is not already loaded, no caching should ever occur because the next time a call to this entry point occurs it may be in a different location.

All common operating system platforms provide for name-based exporting of functions in libraries. In all known cases, these operating systems provide case-sensitive name processing. The name string used for the lookup must match exactly, including case.

If the lookup of the function name fails (the OS reports that there is no such function, usually by returning a NULL pointer), the following 4GL error will be raised:

Could not find the entrypoint <function_name>. (3260)

Please note that the name specified in the RUN statement can be of any case. Progress will match it to the PROCEDURE EXTERNAL name on a case-insensitive basis. But the name specified in the PROCEDURE EXTERNAL statement MUST match the case of the export exactly.

Windows (and OS/2 for those of you history buffs) also allow exporting a function by "ordinal". This is a simple integer (in the case of Windows it must be between 0 and 65535 inclusive, i.e. an unsigned 16-bit integer). In other words, any OS that supports finding by ordinal has a table mapping integers to function pointers for each loaded module.

On Linux (and UNIX in general), exports by ordinal are not supported. However this leads to a strange behavior in the 4GL. PROCEDURE EXTERNAL definitions always must have a name specified, but when the ORDINAL n clause is also specified, then this changes the error processing, even on Linux.

/* the function name exists but the ordinal does not */
procedure ptr_size external "{&libname}" cdecl ordinal 15558:
   def return parameter sz as long.
end.

Calling ptr_size will work fine on Linux, because the name lookup succeeds. Linux will ignore the specified ordinal in this case.

Calling ptr_size will FAIL on Windows, because when the ordinal is specified on Windows, the name is ignored. When the OS reports that the ordinal does not exist, the following error is raised:

Could not find the entrypoint <function_ordinal>. (3259)

For this case:

/* neither the function name nor the ordinal exist */
procedure some-unknown-garbage-function-name external "{&libname}" cdecl ordinal 15557:
end.

On Linux, the name lookup will happen and will fail. But when the error is raised, it will be the 3259 that is raised (including specifying the "ignored" ordinal) instead of the 3260 with the failing name. This just seems like a bug.

On Windows, the name is never tried and the ordinal lookup fails. It raises a 3259 error as you would expect.

On Windows, if ANY function does exist at the given ordinal, it will be called and no 3259 will be raised. Of course, exporting by ordinal is dangerous, especially since compilers/linkers often change the order of function exports without notice over time. Thus exporting by ordinal is quite dangerous, since one just gets back a function but you don't even have a name with which to assure the caller that they are calling the right export. For this reason, exporting by ordinal never gained large popularity and it is really just a legacy thing on Windows now.

One additional issue with function name lookup is a name decoration problem. Some compilers will "decorate" function names based on the calling convention of the function. Sadly, there is no standard for this and each compiler can do something different or slightly different. For example, CDECL convention normally will export functions in an undecorated form (not changed at all from how they are specified in the source code) but it is sometimes possible to see functions exported with an underscore added as a prefix to whatever name was in the source code. For STDCALL it is much worse, as there is a range of different possibilities:

• no decorations
• underscore prefixing
• suffix using at sign + parameter size in bytes as a decimal number (e.g. a function that takes 2 32-bit integers as parms would append an

  @8

• the combination of underscore prefixing and the at + num suffix

The key question here is whether Progress would try different variants of a name (with different combinations of decorations) when it finds the function address. For CDECL, the answer is NO. Progress only tries the name as specified and the 3260 error (and sometimes 3259 on Linux if the ordinal is specified) will be generated. For STDCALL, the answer is YES! So Progress does try to lookup stdcall funcs by decorating the name with BOTH an underscore prefix AND an at + num suffix that they must calculate from the parms. If the the real function in this case has this name: _underscore_prefixed_function_name_with_at_suffix@4 but the PROCEDURE EXTERNAL specified underscore_prefixed_function_name_with_at_suffix, the function will be found. Interestingly, the algorithm is not very sophisticated. If there is already an underscore prefix but no at + num suffix, Progress will still add both the underscore prefix and the at + num suffix and try the lookup again. If that simple augmentation does not work, the 4GL programmer will have to exactly code the name in the PROCEDURE EXTERNAL statement. Note that underscores and the at + num suffix can legally be specified in that statement, so there is no limitations in 4GL syntax there.

LE: Additional testing confirms that the 4GL does not remove leading or trailing whitespace from a function name (whether on a RUN VALUE or on the PROCEDURE " somename" EXTERNAL definition). Since it is generally not possible to export function names with leading, trailing or embedded whitespace, such functions should always cause an error 3260.

Inbound Checking Findings

The use of "inbound" means on the inbound "side" of the API call. In other words these are the checks that occur when going INTO the API call.

Before any attempt is made to load a library or call a function, the 4GL runs through a set of checks on the call's inputs. If these checks fail, an error is raised and there will be no further processing of that RUN statement.

This is the order of error checks on the "inbound" part of the RUN statement processing. Only the 1st failure found is reported (the checking process ends at that point):

1. Unknown value, uninitialized pointer and empty string tests.

Unfortunately, this logic is not very regular. It is littered with corner and edge cases and unexplainable implementation decisions.

If any of the INPUT or INPUT-OUTPUT parameters is set to the unknown value, the following error occurs:

A variable or array element passed as an INPUT or INPUT-OUTPUT parameter to a DLL cannot contain the Unknown value. (12272)

This occurs for INTEGER/INT64/DECIMAL/CHARACTER/MEMPTR INPUT or INPUT-OUTPUT cases (this is different from OUTPUT and RETURN parameters). Strangely, in all these 12272 situations, NO-ERROR only suppresses the message here (and records the error) BUT the error is still raised!

Passing unknown value for a scalar LONGCHAR of any mode (INPUT, INPUT-OUTPUT) will cause an error 12450 to be raised (it is not the funky processing like 12272, but a real error that can be suppressed by NO-ERROR). Passing unknown value for a LONGCHAR array element is like the 12272 error processing above.

When INTEGER/INT64 OUTPUT parameters are set to unknown value, no error occurs, the API is called BUT no value is copied back.
When INTEGER/INT64 RETURN parameters are set to unknown value, no error occurs, the API is called AND the returned value IS copied back.
When DECIMAL OUTPUT or RETURN parameters are set to unknown value, no error occurs, the API is called AND the returned value IS copied back.

When CHARACTER OUTPUT or RETURN parameters are set to unknown value, the 12450 error is raised (the message can be suppressed with NO-ERROR in this case, but the error is still raised!):

A CHARACTER or LONGCHAR variable passed as an OUTPUT parameter to a DLL cannot be the Unknown value or be uninitialised. (12450)

When LONGCHAR OUTPUT parameters are set to unknown value, it just silently returns with no API called and error 12450 raised.
When LONGCHAR RETURN parameters are set to unknown value, it does call the API, no error is raised BUT no return value is copied back.

Using empty string in INPUT/INPUT-OUTPUT/OUTPUT/RETURN CHARACTER variables sort-of works. The API is called but no value is ever copied back for INPUT-OUTPUT, OUTPUT and RETURN types.

Using empty string in LONGCHAR INPUT/INPUT-OUTPUT/OUTPUT variables generates a 12450 error (without calling the API) except on Windows, there is an internal 4GL segmentation fault (in one of the Progress DLLs) when this is processed, the entire process dies and it does NOT die in the called DLL.

Using empty string in RETURN LONGCHAR variables sort-of works. The API is called but no value is ever copied back.

The 4GL docs state that the ABL does not allow you to use return with character or longchar. Some online discussions suggested that from v9 to v10, the runtime changed to generate a runtime error 12200 BUT in v10.2B we see different behavior as noted above.

If MEMPTR OUTPUT parameters are set to unknown value will cause the 3233 error to be raised.
INPUT, INPUT-OUTPUT and OUTPUT MEMPTR parameters passed as uninitialized or with size set to 0 will also cause the 3233 error to be raised.

DLL procedure <called_proc_name> <caller_procedure_filename> using an uninitialised MEMPTR. (3233)

A RETURN MEMPTR parameter set to unknown value abends the 4GL process!
A RETURN MEMPTR parameter passed as uninitialized actually WORKS! It is set to the return address.
A RETURN MEMPTR parameter passed as 0 size abends the 4GL process!

Except for the LONGCHAR array element anomaly noted above, it is believed that the array elements follow the same checks as their scalar counterparts.

2. The mode and type passed (in that order) for each of the parameters is checked left to right (if not specified, INPUT is the mode). The passed modes/types are compared to the procedure's defined modes/types. The first parameter in which a mismatch is found will cause one of these errors:

Mismatched parameter types passed to procedure <called_proc_name> <caller_procedure_filename>. (3230)
Mismatch in the parameter datatypes in DLL procedure <called_proc_name> <caller_procedure_filename>. (3231)

The modes specified in the RUN statement must match exactly with the modes specified in the DEFINE PARAMETER statements (inside the PROCEDURE EXTERNAL). The only exception is that RETURN is specified as OUTPUT when used in the RUN statement. The mode of the parameter is checked before the data type of the parameter. The 3230 error is raised when there is a mode mismatch.

The following is the allowed mappings of 4GL data types to DLL data types:

• INTEGER and INT64 can be passed for BYTE, SHORT, UNSIGNED-SHORT, LONG, UNSIGNED-LONG or INT64
• HANDLE-TO INTEGER and HANDLE-TO INT64 can be passed for INPUT parameters of type BYTE, SHORT, UNSIGNED-SHORT, LONG, UNSIGNED-LONG or INT64 which are defined by the native code as a pointer to the native type (e.g. int*)
• DECIMAL can be passed for FLOAT or DOUBLE
• HANDLE-TO DECIMAL can be passed for INPUT parameters of type FLOAT or DOUBLE which are defined by the native code as a pointer to the native type (e.g. double*)
• CHARACTER and LONGCHAR can be passed for CHARACTER (which in native code is really a reference to char* or an equivalent)
• MEMPTR can be passed for char* so long as you use MEMPTR in the DEFINE PARAMETER
• MEMPTR must be passed for any other kind of pointer (e.g. void* or even a pointer to a pointer like char**)

Some subtle points:

• If you try to use CHARACTER in a RUN statement but you have the DEFINE PARAMETER set to MEMPTR, the 4GL abends without loading the library or calling any API.
• EXTENT parameters are always passed as a pointer (and the types follow the same rules as noted above)
• DECIMAL cannot be interchanged with INTEGER or INT64 (and vice versa)
• HANDLE-TO is silently tolerated but also ignored when specified for any parameter that is already going to be passed as a pointer (INPUT-OUTPUT parms, OUTPUT parms, EXTENT parms, CHARACTER/MEMPTR INPUT parms)
• literals are implicitly INPUT parameters and cannot be otherwise marked explicitly (you can make the INPUT explicit but it is a compile error to mark a literal as INPUT-OUTPUT or OUTPUT)
• Any use of the DATE, DATETIME, DATETIME-TZ, HANDLE, LOGICAL, RAW, RECID or ROWID types causes a 3231 error. In other words, there is no automatic translation of types (even ones like RECID that could easily be converted to integer types).

When there is a datatype mismatch, the 3231 error is raised. A parameter that has both a mode and data type problem will fail with the 3230 error.

3. At this point there are type-specific value checks for each of the INPUT or INPUT-OUTPUT parameters, processed in a left to right order. The first problem encountered raises the error. For example, an uninitialized memptr passed for any of the parms will cause:

DLL procedure <called_proc_name> <caller_procedure_filename> using an uninitialised MEMPTR. (3233)

Likewise, for the integer/int64, overflows/underflows will cause this:

Value -129 does not fit in byte DLL datatype. (13712)

The detection is dependent upon the "DLL" type that was specified in the procedure external statement (which may or may not match what is actually used in the library). Here are the ranges (they are inclusive of the endpoints):

BYTE: -128 through 255
SHORT: -32768 through 32767
UNSIGNED-SHORT: 0 through 65535
LONG: -2147483648 through 2147483647
UNSIGNED-LONG: 0 through 4294967295
INT64: can't overflow or underflow

The decimal 4GL types do not raise errors on overflow or underflow. If it can fit the (most significant) data in with a loss of precision, it will do so. If it can't fit the data at all, then the values will be written as positive or negative infinity (depending on the sign of the input value).

It is OK to pass a CHARACTER value which is empty string, but when LONGCHAR is passed containing empty string, the target API is never called and there is no error raised.

If an EXTENT is passed as an INPUT or INPUT-OUTPUT parameter, for the most part, all elements must pass the same checks.

4. Finally, the number of parameters is checked. If there is the wrong number (too few or too many), then this error is raised (it is the same in either case):

Mismatched number of parameters passed to routine <called_proc_name> <caller_procedure_filename>. (3234)

Polymorphic Types

And another one: I wonder how _POLY cases behave in this case... will a _POLY still get automatically converted to the expected type, if compatible?

Normal 4GL procedures have some level of automatic type morphing, BUT the native API support is much more picky (which is good).

function num-as-text returns char (input num as int):
   return string(num).
end.

run input_int32_cdecl (input "0") no-error.
if not error-status:error or not error-status:get-number(1) eq 3231 then run log-err (input "Expected error 3231 for mismatching parameter type (1). Got '" + error-status:get-message(1) + "'.").

run input_int32_cdecl (dynamic-function("num-as-text", 14)) no-error.
if not error-status:error or not error-status:get-number(1) eq 3231 then run log-err (input "Expected error 3231 for mismatching parameter type (2). Got '" + error-status:get-message(1) + "'.").

{library_calls/libname.i}

procedure input_int32_cdecl external "{&libname}" cdecl:
   define input parameter i as long.
end.

The API is expecting an INPUT parameter that is a number. In both cases we passed a string that was a valid number. This works with regular internal procedures but it is not possible with native API calls. This is consistent with the findings that other common data type interoperability is also disabled for native API calls (e.g. decimal and integers can't be interchanged).

BOTH cases cause:

Mismatch in the parameter datatypes in DLL procedure input_int32_cdecl library_calls/misc/polymorphic_type_support.p. (3231)

Some Calling Convention Findings

STDCALL is really the default calling convention on 32-bit Windows (whether run in a real 32-bit OS or in the WOW environment on 64-bit Windows). If the calling convention is not explicitly specified when running on 32-bit Windows, then STDCALL will be used during dispatching.

In the 32-bit Windows environment, it is VERY IMPORTANT that the calling convention specified on the PROCEDURE EXTERNAL should match the calling convention actually specified in the compiler for the native library code.

For example, when passing integer or floating point data as INPUT parameters, the data is not passed as a pointer. Since the 4GL has a procedure definition that includes parameters of different widths (BYTE versus SHORT), if there is a mismatch in what the 4GL thinks it must pass and the actual size of the data that is expected by the API, this can break the STDCALL processing. STDCALL uses callee cleaning of the stack, so it is very sensitive to any case of mismatched argument sizes. When the 4GL tries using args that are not the same length as is expected by the functions called, the stack pointer will end up pointing to the wrong index (after the proper location when the passed args are larger than expected and before the proper location when the passed args are smaller than expected. In other words, this happens because the size of the args pushed on the stack is different from the amount of stack that the callee cleans. In CDECL, the caller may push the wrong stuff on the stack, but since the caller cleans that same amount, the stack never gets "out of sync". The 4GL code that does the dispatching/calling can become unstable in this case, causing the process to abend with an error like this:

'C' Call Stack has been compromised after calling <called_api_func_name> in <library_name> (6069)

This can happen for INPUT parms. The other parm modes use pointers (INPUT-OUTPUT or OUTPUT) or use registers (e.g. for RETURN data) and thus cannot destabilize the process. This can also happen if the calling convention used by the 4GL is different from that implemented in the library (e.g. STDCALL was implicitly or explicitly specified in the 4GL code but the actual API is CDECL).

No other OS environments support different calling conventions. In other words, all other dispatching is hard coded to the calling convention of the platform even if the code specifies a different calling convention.

Other than Windows 32-bit (which allows CDECL and STDCALL and possibly PASCAL), the calling convention works as follows:

• Windows 64-bit has a single calling convention that is similar to fastcall (it is register based for the first 4 parms)
• Linux 32-bit uses the System V Application Binary Interface (Intel 386 Architecture Processor Supplement)
• Linux 64-bit uses the System V Application Binary Interface (AMD64 Architecture Processor Supplement)
• Solaris 64-bit uses the SPARC v9 Application Binary Interface

No testing of PASCAL was done. It is not known which PASCAL is supported. Newer references to PASCAL in Windows suggest that it is similar to (possibly even the same as) STDCALL (where parameters are pushed right to left). But older references to PASCAL (which was the 16-bit Windows calling convention and was also used in 16-bit OS/2), note that PASCAL pushed parameters left to right. Evidently, really old Borland compilers used that PASCAL version. It is not clear which version of PASCAL is actually supported. But the 4GL docs suggest this is only for compatibility purposes, so it may just be a synonym for STDCALL.

For now, NO support of PASCAL will be provided in P2J.

There is quite a bit of trickiness in supporting these different calling conventions. First of all, these conventions are often only lightly documented. Even when the documentation does exist, it is often incomplete. For this reason, it is difficult to know exactly what the 4GL supports. On a positive note, the 4GL deliberately limits its support to a specific set of data types:

• integer data that can fit in 32-bits
• integer data that can fit in 64-bits
• floating point data that can fit in 32-bits
• floating point data that can fit in 64-bits
• data passed by pointer

This means that many of the more complicated parameter passing conventions are avoided. For example, there is no integration with C++ OO mechanisms (passing an implicit "this" pointer as a first parameter). More important, there is no way to pass a structure by value. Of course the 4GL doesn't really have 'C' structure support, but temp-table records are a close substitute. Many calling conventions have tricky hidden pointers and special "copy values into specific registers" or stack placement approaches to passing larger data by value. These can all be ignored.

But it is important to handle the 32 vs 64 bit width issues properly. With 64-bit calling conventions, the width of each parameter is typically 64-bits on the stack AND usually the pointer size is 64-bits. This means that all of the possible parms naturally are passed in only 1 stack slot (e.g. a single assembly language "push"). On 32-bit platforms, the width of a stack slot is 32-bits. While pointers are normally also 32-bits wide and are safe, passing 64-bit wide data usually requires two stack slots OR 2 registers (one for the high word and another for the low word). This is where things get tricky. The 4GL does support these cases (i.e. 64-bit integer values are supported on 32-bit platforms) including as both arguments and as return values.

All of these conventions use registers for at least some of the parameter passing and for MOST of the return value processing. Different data types are often mapped into different registers (especially on Intel where floating point support is bit of a step-child). There are even some tricky parts here related to which of the caller's registers are saved/restored by the callee.

Then there is variadic argument support (varargs). STDCALL doesn't support it (because it is a callee cleans convention). Luckily, the 4GL also does not support varargs for native calls. All PROCEDURE EXTERNAL definitions have a fixed set of parameter definitions.

With all of this in mind, I have decided to try to use an existing open source library called libffi to implement the actual dispatch of the API call. Some useful references:

https://sourceware.org/libffi/
http://en.wikipedia.org/wiki/Libffi
http://www.manpagez.com/info/libffi/libffi-3.0.13/
https://github.com/twall/jna

The core idea of libffi is that you have a 'C' interface for dynamically defining (at runtime) the signature of a given pointer. Then you can dispatch a call to that pointer with specific parms and get back the return value. This is exactly what we would have to write anyway.

The JNA project is interesting, but it has its own version of how library loading and function lookups occur. There doesn't seem to be any benefit of trying to force fit their approach into the peculiar 4GL behavior. I think it is less work to implement everything ourselves except for the assembly portions that handle the dispatching. By using libffi, we should be able to get a well tested implementation of all the of necessary calling conventions without writing the code ourselves. There are 2 potential downsides:

1. What if their code isn't compatible with some craziness that the 4GL implements? We should know this during development, unless it is a case triggered by a really obscure function signature.

2. We must have a version compiled and installed on every client system we use. The good news is that this is easy to do for all our target platforms. But it is one more dependency.

My assessment is the benefits outweigh the costs, so it is worth the attempt.

Outbound Checking Findings

The use of "outbound" means on the outbound "side" of the API call. In other words these are the checks that occur when coming BACK from the API call.

After the call a library function, the 4GL runs through a set of checks on the call's outputs (if any). If these checks fail, an error is raised and there will be no further processing of that RUN statement.

Please see the inbound checking notes above since variables that were unknown, uninitialized, 0 size or empty can have an affect on whether or not any data is copied back.

Each parameter is processed from left to right, in order. INPUT parameters are ignored because they will never have data copied back. INPUT-OUTPUT, OUTPUT and RETURN parameters are checked to ensure that the data can be copied back (or the failure occurs as part of the copying back process). Only the 1st failure found is reported (the checking process ends at that point). Any RETURN parameter is processed in whatever place it exists in the caller's definition, rather than always first or always last.

The checking itself is data-type specific.

INTEGER processing

For integer data it can be dependent upon 3 factors:

• The vartype which is the 4GL variable type to which we are assigning. This can be either integer or int64. If the vartype is int64, there can't be any errors detected because there can't be an overflow or underflow.
• The apitype which is the 4GL shared library type name (used in the PROCEDURE EXTERNAL parameter definition) that is used to interpret the returned value.
• The rettype which is the actual shared library function type being returned from the native code.

If the vartype is integer, then the following conditions can cause errors to be raised:

• s-overflow = apitype is int64 and rettype is int64
• n-overflow = apitype is int64 and (rettype is either int32 or uint32)
• u-overflow = apitype is unsigned-long and (rettype is int32, uint32 or int64)

The pseudo-code below uses outval to refer to the actual value being copied back and inval for the value being written to the API for INPUT-OUTPUT parms. Each of the below conditions will cause a 15747 error to be raised:

Value <output_value> too large to fit in INTEGER. Line 0 in <called_procedure_name> <calling_program_name>. (15747)                                                                               

Although the output value is normally a valid number, when the returned value is INT64_MIN (-9223372036854775808) it will be reported as -(.

• s-overflow and (outval < -2147483648 or outval > 2147483647)
• (u-overflow or n-overflow) and the outval sign bit is 1

INPUT-OUTPUT parameters:

• s-overflow and (outval < -2147483648 or outval > 2147483647)
• u-overflow and the outval sign bit is 1
• n-overflow and inval is non-negative and the outval sign bit is 1
• n-overflow and inval is negative and the funky-int-mashup-is-out-of-bounds condition is true

The funky-int-mashup-is-out-of-bounds condition occurs when the apitype is int64 and the rettype is int32 or uint32. If the combination of an 0xFFFFFFFF upper doubleword is combined with a lower doubleword being copied back that results in the 64-bit value read back being less than -2147483648 or greater than 2147483647, then it will result in a value that cannot be assigned back to an integer.

The key point here is that INPUT-OUTPUT cases will have different results because of the input bits left untouched by the native code when there is a mismatching definition of apitype and rettype.

For INPUT-OUTPUT and OUTPUT array parameters, the same cases always generate the error 15747 which halts the array processing with the Value xx too large to fit in INTEGER. (15747) error (slightly different form). The array copying seems to be an "internal copying loop" that tries to instantiate integer instances but which can be aborted on encountering the first failure, since only 1 error is recorded in the error-status handle AND there is very little data copied into the array. This is different from the decimal "failures" which just show some messages but don't treat them as error conditions (see below). The interesting thing is that the NO-ERROR does suppress the ERROR condition from being raised here but it doesn't stop the internal copy loop from aborting! RETURN parameters can't be arrays so this doesn't affect them.

DECIMAL processing

When float (32-bit) and double (64-bit) are copied back, they must be converted from the binary floating point representation into the fixed point decimal representation used internally in the 4GL.

Both float and double can store the fractional part of a decimal number to a precision that is far greater than the 10 digits possible in the 4GL. For this reason, if the 11 most significant digits of the fractional portion are zero, then the fractional portion will be set to zero. The "good" news is that such an operation drops that precision silently and no errors are raised.

Where errors can occur, is on very large floating point values. A 32-bit float can hold a value with an exponent of 2^127 which is roughly the same as 10^38.5. In other words, a 32-bit float should have a non-fractional maximum of 39 to 40 digits. Such a value is SUPPOSED to fit into a 4GL decimal. BUT when the largest 32-bit floating point value (0x7f7fffff) is read into a decimal, the 4GL will raise this error:

Insufficient space for float to decimal conversion. (86)

This occurs in GET-FLOAT as well as when reading back into a decimal from a parameter defined as a float. Actually, in the GET-FLOAT case, the 4GL process itself has a memory access violation and aborts hard. Impressive coding there! Of course, we aren't going to duplicate that.

Regardless of the situation, there is no good explanation for why the Progress 4GL runtime causes such an error. Likewise, I don't know for what range or other conditions this error may occur. For now, we won't duplicate this (until we have more details).

When copying a double back into a decimal, the situation is much different. The largest double has an exponent of 2^1023 which is roughly 10^308.25. That is far beyond the 40-50 possible non-fraction digits that can be stored in a 4GL decimal. Numbers that cannot fit will cause this error to be raised:

Decimal number is too large. (536)

This one we can check for and implement. Again, we don't know if the behavior is fully regular, but the basic idea should be straightforward.

The other special floating point cases convert strangely as well.

• The 4 possible NaN (Not a Number) values are quiet NaN, -quiet NaN, signaled NaN and -signaled NaN. All 4 of these convert to decimal zero.
• The 2 possible infinity values (positive and negative) both cause the 4GL to go into an infinite loop! Ironic, right? The 4GL literally goes into an infinite loop in its internal processing that is trying to convert to decimal. This happens in GET-FLOAT, GET-DOUBLE and in the copying back of float or double parameters to decimal. This infinite loop cannot be stopped by CTRL-C. In fact, the loop will consume 100% of the CPU for that thread. You have to hard kill the process at the OS level. We won't be duplicating this infinite loop behavior. Another impressive feat of software engineering by the computer scientists at PSC.

INPUT-OUTPUT and OUTPUT decimal arrays (RETURN parms can't be arrays):

Calling a function that uses decimal arrays can cause numerous messages to be displayed if run without NO-ERROR. The messages are not treated as "errors" (even without NO-ERROR), so the ERROR condition is not raised and when NO-ERROR is used there is no update of the ERROR-STATUS list of errors (well, the list isn't updated w/o NO-ERROR either, but that is normal).

In our INPUT-OUTPUT tests there were 4 messages generated (error 86). The problem only occurs calling input_output_double_cdecl_limit_array() when specifying the parameter type as float. By changing the output values that the DLL writes back it was found that these "complaints" are only caused by some combination of the data being written back, possibly in combination with the data that was input as well. The important points here:

1. If safe values were written back (e.g. 0) then there would not be any complaints. There are no errors on INPUT.
2. When we write only a 1 element array (actually it is 2 elements but the DLL is told that it is only 1 element long), there are many fewer complaints than occur in this same situation in the single big array call in input_output_float_test.i.

In a different version of the INPUT-OUTPUT tests, we found many (10s) 'complaints' which are all one of these three (the 3rd one has no real error text, just the number!):

Insufficient space for float to decimal conversion. (86)
Decimal number is too large. (536)
15747

In our OUTPUT decimal array tests, we only saw messages that were one of these two:

Insufficient space for float to decimal conversion. (86) */
Decimal number is too large. (536) */

The problem seems to be some mismatch with the 4GL using float data types and the native side using double. However, there are even a large number of problems reported in the double/double case and in native float API + 4GL using double. The float/float case is the only one that has no messages. As shown during testing for input_output_float_errors.i, there are no errors on INPUT, all errors are caused by some combination of the data being written back from the DLL.

CHARACTER/LONGCHAR processing

Please see the INPUT checking notes above for how empty string and unknown value are processed. Those cases do have an affect on the results of OUTPUT processing.

The 4GL docs state that the ABL does not allow you to use output with character or longchar, BUT it turns out that it works fine on v10.2B.

For RETURN parameters, the 4GL docs state that the ABL does not allow you to use return with character or longchar. There are some online dicsussions that suggested that from v9 to v10, the runtime changed to generate a runtime error 12200 BUT in v10.2B we see a silent execution with no data changed. The APIs are called in this case, there is just no value copied back!

When passing as INPUT-OUTPUT or OUTPUT the CHARACTER/LONGCHAR variables must have enough data already in them to hold the changes otherwise you might see an error like:

Your LONGCHAR buffer has been overrun.  Memory has been corrupted. (14528)

It is likely that the 4GL runtime will detect that the null character is not within the pre-allocated space and then generate this error.

Other than the above length limitations (and the behavior of empty/unknown vars), there is no other check done. The data is copied back based on finding the null character. As you would expect, the null character is not present in the 4GL var after the copy (and it is appended to the copied data on INPUT) by the runtime.

MEMPTR processing

When passing as INPUT-OUTPUT or OUTPUT parms, we must pre-allocate the buffers passed, otherwise we get this:

DLL procedure <native_function_name> <calling_procedure_filename> using an uninitialised MEMPTR. (3233)

For RETURN parms, the 4GL will automatically initialize the memptr to point to the DLL memory that represents the returned pointer (which may point to a null-terminated string or some other buffer). DO NOT de-allocate this MEMPTR since the 4GL did not allocate the memory, it should not try to free the memory. The actual MEMPTR address (obtained via GET-POINTER-VALUE) will be the SAME as the pointer returned by the library.

When passing arrays of MEMPTR (only possible for INPUT, INPUT-OUTPUT and OUTPUT, not for RETURN), the following rules apply:

• 0 sized and/or uninitialized memptr elements are passed as a null pointer (no matter what the mode).
• For INPUT and INPUT-OUTPUT parms, passing any one (or more, including all) of the elements as unknown value will cause an INPUT error 12272 to be raised. Strangely, NO-ERROR only suppresses the message here (and records the error) BUT the error is still raised! Of course, the API is not ever called in this case.
• For OUTPUT parms, passing any one (or more, including all) of the elements as unknown value will cause the corresponding element to be a null pointer. The API IS called, even if the entire array only has null pointers. On return, the memptr will still be unknown value.
• Allocated memptrs are passed with the actual (internal) memory address as the pointer value (sized as the proper 32-bit or 64-bit width for the native application environment).

Any processing of "pointers to pointers" (e.g. char** or void** or int**) is done "manually" by the application code explicitly processing memory addresses and copying them into the buffer allocated for a memptr (which must be large enough to hold the width of the pointer itself). The 4GL runtime does nothing to support such use cases, any more than it does for 'C' structures. In other words, the 4GL runtime does nothing but allow you to edit and manage your own memory buffers and their contents. If you happen to write a valid memory address into the buffer, that is up to you. No use of HANDLE-TO or anything else in the 4GL will cause something to be passed as a pointer to a pointer.

As mentioned previously, the HANDLE-TO option is only honored for INPUT parms. It is silently ignored for all other types.

RETURN Parameter Additional Notes

The definition of a RETURN parameter can be placed as the 1st parameter definition, last parameter definition or anywhere in between. It does not change how the function signature is handled or how the call is made. It only provides a specification that the 4GL must copy back the return value into a specific variable. The order does not matter, except that it must match the order of the parameters passed in the RUN statement.

In the RUN statement, one must pass an "extra" OUTPUT parameter in the same place as the RETURN parameter was specified in the PROCEDURE definition. Although the 4GL needs that placement to match, it has no affect on the parameters actually passed to the native function. In other words, a PROCEDURE EXTERNAL statement that only differs by the order in which the RETURN parameter is specified, will call the native function in exactly the same way (since the RETURN parameter is never actually passed as a parameter). I don't know why one doesn't specify the parameter as RETURN in the RUN statement, it is just a syntax quirk.

You can only define 1 RETURN parameter in a given PROCEDURE EXTERNAL statement (otherwise it is a compile error).

It is valid to omit the RETURN parameter from a PROCEDURE EXTERNAL statement, and it does not change the resulting call except that the return value is ignored since there is no defined parameter into which to copy back the value. In such a case, it would be an error to pass an extra OUTPUT parm in the RUN statement.

RETURN parameters cannot be defined with an EXTENT (it is a compile error).

An EXTENT variable cannot be passed (in a RUN statement) for a RETURN parameter. The result is:

You cannot pass an array variable for the RETURN parameter of a DLL. (12270)

As mentioned before, EXTENT parameters are always passed as a pointer. One cannot pass an extent var of indeterminate size. Basically, the 4GL allocates a memory buffer sized by (element_size * num_array_elements). Then each element of the array is copied into its own slot in the memory buffer. Then the address of the main buffer is passed as the parameter. This is exactly how 'C' language arrays work.

character, longchar and memptr types are always passed by pointer and for these types, the "element size" is the same as pointer size. In each case it is the pointer that is copied in as the element. In C this is treated as a "pointer to a pointer" which is how an array of pointers works.

Interestingly, since the EXTENT keyword cannot be placed in the DEFINE PARAMETER statement inside a PROCEDURE EXTERNAL, the 4GL knows to do this processing by inspecting the passed argument itself. If that variable is an EXTENT var, then the 4GL automatically handles the conversion to a set of pointers as noted above.

It would be a big problem to pass a scalar var to this same procedure because the 4GL would not pass it as an EXTENT, since it can only rely upon the argument itself for this determination. This is a poor design because it is so fragile.

Misc Notes

The 4GL doesn't use native language header files or other definitions. No common OS platforms provide any introspection or reflection for native APIs. Because of these facts, the design of this mechanism is fragile since any errors in definitions can cause the results to be unpredictable. In fact, problems in the API definitions might only be visible on specific hardware (e.g. endianness issues), specific operating systems (e.g. calling conventions) or with specific data patterns passed (e.g. parameter type/size mismatches). This means that the same definition may mostly work and then only sometimes fail.

The same API can be defined differently in different procedures but only 1 definition can be used in a given procedure. This might work OK for some APIs that support varargs, as an example. Please note that the 4GL does not support varargs any other way and it doesn't really even support it using multiple definitions since the 4GL side must always call the RUN statement with the parameters (in number, type, mode and order) fixed as defined in the PROCEDURE EXTERNAL statement.

Question: what does the NativeProcedureCaller.invoke() return value mean/what is it used for? It is not clear from the javadoc, what I should return there.

Greg Shah wrote:

Question: what does the NativeProcedureCaller.invoke() return value mean/what is it used for? It is not clear from the javadoc, what I should return there.

The return value is actually used only by function calls (and is imposed by the InternalEntryCaller.invoke API). For native calls is OK to return null, as this acts like a procedure call.

BTW, the native invocation code I think looks better in a distinct class; NativeProcedureCaller.invoke should delegate to that.

BTW, the native invocation code I think looks better in a distinct class; NativeProcedureCaller.invoke should delegate to that.

Don't worry. I am just calling into the NativeInvoker class and doing everything else there.

In regard to SourceNameMapper.Parameter.getParameterModes(), is there any reason that I need to "hide" the RETURN mode? The old conversion in collect_names.rules translated RETURN to OUTPUT. We can't do that anymore, because I need to know exactly which of the passed in OUTPUT parameters should be mapped as the RETURN value. But I recall that the 4GL may be reporting the RETURN as an OUTPUT in its signature? And the getParameterModes() would also need to be changed, I think. My general inclination is to code it as an 'R' in getParameterModes() if that is OK and change buildSignature() to report RETURN as OUTPUT. Will that be OK from your perspective?

Greg Shah wrote:

BTW, the native invocation code I think looks better in a distinct class; NativeProcedureCaller.invoke should delegate to that.

Don't worry. I am just calling into the NativeInvoker class and doing everything else there.

In regard to SourceNameMapper.Parameter.getParameterModes(), is there any reason that I need to "hide" the RETURN mode? The old conversion in collect_names.rules translated RETURN to OUTPUT. We can't do that anymore, because I need to know exactly which of the passed in OUTPUT parameters should be mapped as the RETURN value. But I recall that the 4GL may be reporting the RETURN as an OUTPUT in its signature?

Correct, RETURN is reported as OUTPUT by get-signature.

And the getParameterModes() would also need to be changed, I think. My general inclination is to code it as an 'R' in getParameterModes() if that is OK and change buildSignature() to report RETURN as OUTPUT. Will that be OK from your perspective?

The RETURN parameter is marked as OUTPUT by the caller, right? At this time, the returned value by getParameterModes is used in parameter mode validation, by directly comparing the two param mode strings (the requester's and the target's). I think is easier to let getParameterModes return 'O' for any 'R', and if you need the real modes (in NativeInvoker), call a different API which returns the parameter modes string unchanged.

I will go ahead and return 'O' instead of 'R' for getParameterModes().

The mode checking for native API calls is done after some other input checking (for unknown value, empty string and uninitialized memptrs). For this reason, I was planning to handle all mode checking in NativeInvoker.

What you are suggesting is that the native API calls will be mode checked before the NativeInvoker is called, which will be a problem.

Greg Shah wrote:

I will go ahead and return 'O' instead of 'R' for getParameterModes().

OK. This will allow ControlFlowOps.validArguments (the part that checks parameter modes, line 4179) to pass.

For this reason, I was planning to handle all mode checking in NativeInvoker.

I guess you will be overriding InternalyEntryCaller.valid in NativeProcedureCaller? If you want all validation to be done by the NativeProcedureCaller.invoke() call, NativeProcedureCaller.valid should return a ArgValidationErrors.DEFER_VALIDATION and change ControlFlowOps.validArguments so that it will return true when DEFER_VALIDATION is encountered.

Constantin: What is the easiest way to determine if a given BDT[] instance is an indeterminate length array? I have to raise an error if ever such a thing is passed as a parameter to native code.

Greg Shah wrote:

Constantin: What is the easiest way to determine if a given BDT[] instance is an indeterminate length array? I have to raise an error if ever such a thing is passed as a parameter to native code.

Use the ArrayAssigner.isDynamicArray(BDT[]) API.

Eugenie: please review the filesys.c change as well as the other native code changes.

I'm OK with changes. The only question is what about ffi.h header? Do you have them in your min-gw? I can not find them there(neither in my usual headers in Ubuntu). Some special package need to be installed?

I'm asking because I have one point for calling conventions that can be important. The so called "fastcall"(when the part of the parameters are passing via registers first and the rest - on the stack) is obsolete and should be ignored?

The only question is what about ffi.h header?

On Ubuntu, install this package:

sudo apt-get install libffi-dev

On Windows, we will probably have to build it from source:

https://sourceware.org/libffi/

The so called "fastcall"(when the part of the parameters are passing via registers first and the rest - on the stack) is obsolete and should be ignored?

On 32-bit Windows, Progress doesn't support FASTCALL as an option.

Interestingly, the 64-bit Windows calling convention is a derivative of FASTCALL (I think there are some differences), but it doesn't matter so much because it is the only calling convention allowed on 64-bit Windows. FFI should already be setup to handle the calling conventions we need.

• collect_names.rules
  - line 406: this <rule>downPath("KW_OUTPUT") or downPath("KW_RETURN") should be <rule>downPath("KW_OUTPUT") (as KW_RETURN is explicitly treated downstream)
• ControlFlowOps:
  - release(character): you have a libname.isUnknown check at the begining of the method, but later on you have a libname == null test. This suggests that libname may be null, which would cause a NPE at the libname.isUnknown. Also, I think release(String) should check for null too, before calling NativeInvoker.release.
  - about this code in release(character):
  

        if (libname.isUnknown())
        {
           // this is a cases where silent error mode may be enabled but we must throw the
           // error instead of suppressing it because the 4gl does it that way; in this unusual
           // case the normal pairing of disable calls in the business logic will be broken
           // unless we turn it off here (TODO: is this correct or do we have to actually
           // disable it in the business logic?)
           if (ErrorManager.isSilent())
              ErrorManager.silentErrorDisable();
  
           throw new ErrorConditionException("Unknown cannot be given for a libname!");
        }
  

  
  I don't think the implementation of unknown libname is correct. A test like:
  

  def var ch as char.
  ch = ?.
  release  external ch no-error.
  message error-status:error error-status:num-messages
  error-status:get-message(1).
  

  
  seems consistent with some cases I found in SOAP implementation: the ERROR-STATUS:ERROR flag does get set, but no error message is recorded; and if NO-ERROR is missing, an ERROR condition is raised. I think the correct implementation is similar to SOAPFactory.throwError() - maybe throwError should be moved to ErrorManager.
  I think this might apply also to the TODO in NativeInvoker.invoke:176
  - have you checked how RETURN-VALUE function is affected by a native call? A code like this can be used:
  

  procedure foo.
  return "bogus".
  end.
  run foo. /* this sets return-value to bogus */
  run nativeproc. /* this executes some native procedure which has a RETURN parameter. */
  message return-value. /* is this still "bogus" or something else? */
  

• InternalEntry
  - getParameterList: To avoid returning a copy of the list, we should freeze the parameters (via Collections.unmodifiableList), once they are build by SourceNameMapper.buildParameters. This way, is safe to expose the reference and any modifications to the list will be disallowed.
  - setParameterList: this one I think should be made package private. Only SourceNameMapper is allowed to call it, thus is safe to save the reference.
• BaseNativeType.readExternal - data is read from the stream, but not restored to the instance fields... is this by intent?
• in the Native* subclasess, there is code like this:
  

     protected void readNative(memptr ptr)
     {
        integer num = ptr.getLong(1);
  
        if (!num.isUnknown())
        {
           value = num.longValue();
        }
     }
  

  
  Is it really safe to let value unchanged, if an unknown value is read from memptr?
• NativeInvoker.checkModeAndType: the javadoc is misleading, I think is from another checkUnknown API.
• NativeInvoker.initializeBriefcase: in case of arrays, I think we should check that the element type of the java array is not BDT. Allowing BDT[] instances with a BDT element type (And not a specific sub-class, as character, integer, etc), could pose problems when writing converted code by hand, as the element's size may not be the same. Or maybe check that all elements of the BDT array are of the same subclass (unknown values may be ignored)?
• NativeInvoker.createScalarInstance - the avoid NPE, we will fail downstream comment: if you check the downstream usage, there is no failure related to null arguments in the briefcase. The next usage of argument is in inboundProcessing:267 code: this in turn calls checkUnknown(pname, iename, parm.getMode(), signature[i], args[i], bnt), but checkUnknown has no NPE protection for the bnt.

The CTRL-C 3-way tests failed (expected) and there were 5 failures in tc_job* and tc_dc* tests (1 of which always fails and the others are common false-negatives). I reviewed the unexpected failing tests and none had any failures that looked real or otherwise related to this update. Based on this I am considering that the 0123b update has passed runtime regression testing. The test results have been archived in 10446_f202309_20140123_ges.zip on the shared drive.

Checked in as bzr revision 10447 and distributed.

Building on Windows notes:
The first thing after including ffi.h into compilation preocess is to change the library_win.c from:

...
for (int i = 0; i < len; i++)
...

...
int i;
for (i = 0; i< len; i++)
...

The second and main is compilation of the libffi.lib in Windows itself. Yes, we have the sources. But it is difficult to accept to be compiled with MinGW. There is no target for ./configure in Linux for MinGW build in Windows. Looks like we need to make all preparation steps manually.

The other way is to use precompiled binaries. For example can be taken from here: http://hexchat.github.io/gtk-win32
This particular build is not properly linked with p2j native due to several errors for unrecognized chars inside, so looks like it was built with incompatible versions of C compiler(VisualC++ or Cygwin maybe). Continue investigations.

Eugenie: it is OK to get the compilation issues and the libffi.dll built properly. But after that, pause your work on this task. I have many pending fixes to the code that are required for it to work on ANY platform. So I don't want you to be debugging things I have already fixed or that I am currently fixing.

Eugenie: it is OK to get the compilation issues and the libffi.dll built properly. But after that, pause your work on this task. I have many pending fixes to the code that are required for it to work on ANY platform. So I don't want you to be debugging things I have already fixed or that I am currently fixing.

Greg, OK. The problem is we can not compile p2j.dll in Windows with libbffl.lib binary due to the following errors:

...
native:
     [exec] gcc -o p2j.dll process.o filesys.o memory.o terminal.o library.o init.o signals.o process_win.o filesys_win.o terminal_win.o library_win.o registry.o signals_win.o -static-libgcc -shared -llibffi -lmsvcrt -lgdi32 -Wl,--subsystem,windows,--kill-at
     [exec] Warning: .drectve `/DEFAULTLIB:"MSVCRT" /DEFAULTLIB:"OLDNAMES" ' unrecognized
     [exec] Warning: .drectve `/DEFAULTLIB:"MSVCRT" /DEFAULTLIB:"OLDNAMES" ' unrecognized
     [exec] Warning: .drectve `/DEFAULTLIB:"uuid.lib" /DEFAULTLIB:"uuid.lib" /DEFAULTLIB:"MSVCRT" /DEFAULTLIB:"OLDNAMES" ' unrecognized
     [exec] c:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/4.8.2/../../../../lib/libffi.lib(src/x86/.libs/ffi.obj):(.text$mn+0x6e): undefined reference to `__chkstk'
     [exec] c:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/4.8.2/../../../../lib/libffi.lib(src/x86/.libs/ffi.obj):(.text$mn+0xae): undefined reference to `__security_check_cookie'
     [exec] c:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/4.8.2/../../../../x86_64-w64-mingw32/bin/ld.exe: c:/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/4.8.2/../../../../lib/libffi.lib(src/x86/.libs/ffi.obj): bad reloc address 0x1e in section `.text$mn'
     [exec] collect2.exe: error: ld returned 1 exit status
     [exec] make: *** [p2j.dll] Error 1
...

Do you have the compilation fixes ready now? If so, please upload them for review.

Have you resolved the way to build a fresh libffi for Windows? If so document it here and attach the properly built version (in a separate update zip).

Code Review evl_upd20140127.zip

The change is fine. Check it in and distribute it. Then post the revision number here. This doesn't need any regression testing.

Conversion regression testing for ges_upd20140127a.zip has passed. No change occurs in the output. This has been checked in as bzr revision 10455.

The update evl_upd20140127a.zip committed in bzr as 10456. No regression testing required, only Windows syntax code was touched.

Greg, the question. What if to build libffi in mingw we have to set up MSYS? Is it acceptable?

What if to build libffi in mingw we have to set up MSYS? Is it acceptable?

Yes. But we would only use that environment for building libffi. We don't want to add that requirement to the p2j build process.

Greg Shah wrote:

...
Ovidiu: please review the rules/include/* changes, in regard to assign trigger var def support. Also please look at the changes in int64, integer and decimal.

I used the ges_upd20140123b.zip for review as it was the last with respective changes before getting into bzr.

report.rules:1475
The rule should be:

   <rule>
      ttype == prog.kw_old                              and
      target.upPath("STATEMENT/KW_ON")                  and
      target.parent.descendant(1, prog.db_event))       and
      target.parent.descendant(2, prog.kw_assign))

      <action>tname = "ON Stmt Database ASSIGN Trigger"</action>
   </rule>

   <rule>
      ttype == prog.kw_old                              and
      target.upPath("STATEMENT/KW_ON")                  and
      target.parent.downPath(DB_EVENT/KW_ASSIGN))

      <action>tname = "ON Stmt Database ASSIGN Trigger"</action>
   </rule>

in order to make the difference from WRITE trigger that also have an syntax like:
on write of <table> old <old-table>

report.rules:2370
Sequence functions and statements look good.

int64.java
integer.java
Nothing suspicious.

• typo: trunction
• 3rd parameter is constant: true
• 2nd parameter is also constant: false, however, at the recursive call at line 1709 the 2nd parameter probably should have true (it make sense as stripTrailingZeros() was called on 1st param)
• because left = total - fract, total - fract at line 1693 is left and left + fract at line 1701 is also total
• the following P4GL procedure
  

     DEFINE VARIABLE d AS DECIMAL.
     d = 10000000000000000000000000000000000000123456789121 + 0.0000000002 .
     MESSAGE d.
  

  
  will trigger an infinite recursive call (should be ** Decimal number is too large. (536)). In fact this will work properly if the recursive call uses true as 2nd param.

common-progress.rules
I did not yet finished checking it. There aren't many changes, but there might be some hidden trigger-related bug. I need further analysis with some sample files.

... continued
common-progress.rules
is good except for the fact that maybe we should make the condition at line 1753:
execLib("get_db_event_type", target.parent) != null
to became stronger by checking the result:
execLib("get_db_event_type", target.parent).equals("DatabaseEventType.ASSIGN")
On the other hand, looking at the calling graph I did not see any possibility that the function to be called form other type of trigger than Assign so I guess testing with null should be enough.

or better:

   <rule>
      ttype == prog.kw_old                              and
      target.upPath("STATEMENT/KW_ON")                  and
      target.parent.downPath(DB_EVENT/KW_ASSIGN))

      <action>tname = "ON Stmt Database ASSIGN Trigger"</action>
   </rule>

I made this change to reports.rules.

typo: trunction

Fixed.

3rd parameter is constant: true

This is true when called from decimal, but it is false when called from NativeInvoker.

2nd parameter is also constant: false, however, at the recursive call at line 1709 the 2nd parameter probably should have true (it make sense as stripTrailingZeros() was called on 1st param)

Yes, this was a bug. Thanks for seeing it!

It turns out that my testcases also found this one.

because left = total - fract, total - fract at line 1693 is left and left + fract at line 1701 is also total

Really good feedback. I have made these changes and the resulting code reads MUCH better.

to became stronger by checking the result:
execLib("get_db_event_type", target.parent).equals("DatabaseEventType.ASSIGN")

I made this change too.

I'll be posting my final update shortly.

As can be seen from the logs, the remaining diffs are:

1. A missing error 86 in one call to output_float_from_buffer_cdecl() when reading 0x7f7fffff. This is a "feature" of the 4GL which doesn't make any sense. We can't understand why any float value being read into a decimal would ever cause an error 86, since the decimal can hold the largest possible float. So this deviation is intentional.

2. The log files have the current working directory printed out. This is expected to be different between systems.

3. Floating point values:

• Some of the input_output_double_cdecl(), input_output_float_cdecl(), input_output_float_cdecl_limit_array(), input_float_cdecl() and input_float_cdecl_limit_array() cases show slight deviations in written values. This may be due to slight differences in the IEEE 754 implementation in Java versus the 4GL. None of these differences seem significant. I don't think these differences affect common cases, so we are going to leave this for now. Almost all of these also only occur when passing a double into a float or a float into a double. In other words, when the data type as coded in the 4GL is mismatching with the actual datatype of the library, the code seems a little more sensitive to some differences in behavior. This may be due to sensitivity to stack/register values, as some of these same cases can appear to be different even between 2 sequential P2J runs on the same system.
• There is a set of calls to input_double_cdecl() that all fail, in the "input_parameter_decimal_cdecl with float" test. Only the non-array cases fail, but they all fail. Again, this is a pretty obscure set of mismatching cases.

4. The input_int_parameter_into_double_is_nop output section is expected to differ as it is dependent upon the state of the stack/registers and the output is expected to be junk. This will differ even between 2 sequential P2J runs on the same system.

These are all accepted at this time and no further work will be done on them. These same differences are expected on Windows and they are not considered issues to resolve there either.

All testcase updates and captured logs/output are checked into the testcases project.

The code has passed conversion regression testing. Runtime regression testing is executing now.

This update is merged up to bzr revision 10466. I am going back through runtime regression testing. The first run failed due to the P2J version being slightly back level when my update was applied.

Between 2 regression testing runs, all tests have passed except for GSO 167 (and TC JOB 002 which fails as expected in the text file comparison). I manually tested GSO 167 and it works. Runtime regression testing has passed.

Checked into bzr as revision 10467.

Greg,

The question about task #2234. The goal is to take the testcases from native_library_calls_4gl_..._sample_20131107.zip and ensure the test_runner.p and all dependent sources are converted and executed as expected in Windows. Correct?

Almost.

take the testcases from native_library_calls_4gl_..._sample_20131107.zip

No, that is just a slightly older snapshot of the testcases. You should just get the latest by a bzr checkout of the testcases project. All of this code is in the uast/library_calls/ directory.

1. You can convert them using this:

cd <path_to_testcases_dir>/testcases/uast/
ant clean && ant convert-all -D4gl.file.list=library_calls/cvt_file_list.txt && ant jar

2. Build and install the p2j.dll using the instructions in library_calls/testapi/readme.txt.

3. Start the P2J server like this (make sure that persistence is enabled in the directory.xml):

cd <path_to_testcases_dir>/simple/server/
./server.sh -d -z<path_to_testcases_dir>/testcases/uast/build/lib/testcases.jar:<path_to_testcases_dir>/testcases/uast/p2j/build/lib/p2jadmin.jar:<path_to_testcases_dir>/testcases/uast/p2j/build/lib/p2j.jar:.

4. You can run the testcases like this:

cd <path_to_testcases_dir>/testcases/uast/
../simple/client/client.sh -d2081 -z<path_to_p2j_dir>/p2j/build/lib

ensure the test_runner.p and all dependent sources are converted and executed as expected in Windows. Correct?

Yes, exactly.

Trying to start the server on Windows for native library with persistence enabled. But the error is happening.

Ti enable persistence for server the following changes should be done in directory.xml file:
...
<node class="container" name="persistence">
<node class="boolean" name="foreign-keys">
<node-attribute name="value" value="FALSE"/>
</node>
<node class="boolean" name="active">
<node-attribute name="value" value="TRUE"/>
</node>
</node>
...
Correct?

The error log is:
...
[02/27/2014 16:36:16 PST] (com.goldencode.p2j.persist.DatabaseManager:INFO) Using H2 database version 1.3.169 (2012-09-09)
[02/27/2014 16:36:17 PST] (com.goldencode.p2j.persist.TempTableConnectionProvider:INFO) Temp table connection provider configured; backed by com.goldencode.p2j.persist.UnpooledConnectionProvider
[02/27/2014 16:36:19 PST] (org.hibernate.engine.jdbc.spi.SqlExceptionHelper:WARN) SQL Error: 90008, SQLState: 90008
[02/27/2014 16:36:19 PST] (org.hibernate.engine.jdbc.spi.SqlExceptionHelper:ERROR) Invalid value "en_US_P2J" for parameter "collation"; SQL statement:
set collation "en_US_P2J" [90008-169]
[02/27/2014 16:36:19 PST] (org.hibernate.engine.jdbc.spi.SqlExceptionHelper:WARN) SQL Error: 90008, SQLState: 90008
[02/27/2014 16:36:19 PST] (org.hibernate.engine.jdbc.spi.SqlExceptionHelper:ERROR) Invalid value "en_US_P2J" for parameter "collation"; SQL statement:
set collation "en_US_P2J" [90008-169]
com.goldencode.p2j.cfg.ConfigurationException: Initialization failure
at com.goldencode.p2j.main.StandardServer.hookInitialize(StandardServer.java:1522)
at com.goldencode.p2j.main.StandardServer.bootstrap(StandardServer.java:723)
at com.goldencode.p2j.main.ServerDriver.start(ServerDriver.java:423)
at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:423)
at com.goldencode.p2j.main.ServerDriver.process(ServerDriver.java:150)
at com.goldencode.p2j.main.ServerDriver.main(ServerDriver.java:731)
Caused by: java.lang.RuntimeException: Error initializing persistence services
at com.goldencode.p2j.persist.Persistence.initializeInstance(Persistence.java:3674)
at com.goldencode.p2j.persist.PersistenceFactory.getInstance(PersistenceFactory.java:109)
at com.goldencode.p2j.persist.DatabaseManager.initialize(DatabaseManager.java:1313)
at com.goldencode.p2j.persist.Persistence.initialize(Persistence.java:1081)
at com.goldencode.p2j.main.StandardServer$7.initialize(StandardServer.java:919)
at com.goldencode.p2j.main.StandardServer.hookInitialize(StandardServer.java:1518)
... 5 more
Caused by: org.hibernate.exception.GenericJDBCException: Invalid value "en_US_P2J" for parameter "collation"; SQL statement:
set collation "en_US_P2J" [90008-169]
at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:54)
at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:125)
at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:110)
at org.hibernate.engine.jdbc.internal.proxy.AbstractStatementProxyHandler.continueInvocation(AbstractStatementProxyHandler.java:129)
at org.hibernate.engine.jdbc.internal.proxy.AbstractProxyHandler.invoke(AbstractProxyHandler.java:81)
at com.sun.proxy.$Proxy5.executeBatch(Unknown Source)
at com.goldencode.p2j.persist.embed.H2Helper.executeDDL(H2Helper.java:393)
at com.goldencode.p2j.persist.embed.H2Helper.prepareDatabase(H2Helper.java:114)
at com.goldencode.p2j.persist.Persistence.initializeInstance(Persistence.java:3664)
... 10 more
Caused by: org.h2.jdbc.JdbcBatchUpdateException: Invalid value "en_US_P2J" for parameter "collation"; SQL statement:
set collation "en_US_P2J" [90008-169]
at org.h2.jdbc.JdbcStatement.executeBatch(JdbcStatement.java:634)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.hibernate.engine.jdbc.internal.proxy.AbstractStatementProxyHandler.continueInvocation(AbstractStatementProxyHandler.java:122)
... 15 more

Regarding previous note error. Do I need to set up the SQL server running on Windows to execute the tests?

Eugenie Lyzenko wrote:

Ti enable persistence for server the following changes should be done in directory.xml file:

Correct.

I think what you are missing is copy the p2jspi.jar in your JRE's ext lib folder.

Eugenie Lyzenko wrote:

Regarding previous note error. Do I need to set up the SQL server running on Windows to execute the tests?

No, port redirection is enough.

I think what you are missing is copy the p2jspi.jar in your JRE's ext lib folder.

Yes, you are right, the server is now started.

I'm executing test_runner.p in both Linux and Windows to compare behavior/results. The Linux copy has the error I need to clarify:

...
** "  ptr_size" was not found. (293)
...

...
public class FindByNameFailureStdcall
{
...
            ControlFlowOps.invokeWithMode("  ptr_size", "O", i);
...
            ControlFlowOps.invokeWithMode("ptr_size  ", "O", i);
...
}
...
public class FindByNameFailureCdecl
{
...
            ControlFlowOps.invokeWithMode("  ptr_size", "O", i);
...
            ControlFlowOps.invokeWithMode("ptr_size  ", "O", i);
...
}
...

The question is if the strings " ptr_size" and "ptr_size " are correct converted code? Looks a bit strange. Why not just to use "ptr_size" without extra spaces?

The Linux copy has the error I need to clarify:

Please compare your results to the logs in note 79 to see if errors are expected or not.

The question is if the strings " ptr_size" and "ptr_size " are correct converted code? Looks a bit strange. Why not just to use "ptr_size" without extra spaces?

Please look at library_calls/misc/find_by_name_failure.i for details. The short answer: yes, this is intentional:

/* Could not find the entrypoint   ptr_size. (3260) */
run value("  ptr_size") (output i) no-error.
if not error-status:error or not error-status:get-number(1) eq 3260 then run log-err (input "Expected error 3260 for embedded leading whitespace in external function nam
e. Got '" + error-status:get-message(1) + "'.").

/* Could not find the entrypoint ptr_size  . (3260) */
run value("ptr_size  ") (output i) no-error.
if not error-status:error or not error-status:get-number(1) eq 3260 then run log-err (input "Expected error 3260 for embedded trailing whitespace in external function name. Got '" + error-status:get-message(1) + "'.").

The first thing I've faced is the issue with hardcoded test file path name to run for example:

...
run library_calls/load_unload/load_unload_tests.p.
...
converting to Java as:
...
ControlFlowOps.invoke("library_calls/load_unload/load_unload_tests.p");
...

...
IF WINDOWS
run library_calls\\load_unload\\load_unload_tests.p.
ESELIF LINUX
run library_calls/load_unload/load_unload_tests.p.
...

In the 4GL, this code works without problem on both Linux and Windows.

This means there is a runtime problem in our code. That problem must be fixed.

Or even more, if we expect the program to be run on both Linux and Windows - the special preprocessing directives are adding to the source *.p file to separate OS specific code, something like:

No, this is not a conversion problem. We can't change the source code since it is valid 4GL and should work at runtime on either platform.

Constantin will probably have some thoughts on this. It is really not a problem that is specific to #1634. It is just a Windows platform support issue.

Another similar issue is library name transformation from requested "library_calls\\testapi\\testapi.dll" to "library_calls~\testapi~\testapi.dll" during runtime. As the result - the library can not be loaded in Windows because "~\" is not a valid file separator.

Is this point also out of the scope of the current task?

Outside of this issue I have the test executed in Windows and results to analyze.

Greg Shah wrote:

No, this is not a conversion problem. We can't change the source code since it is valid 4GL and should work at runtime on either platform.

This is only one way: using the Windows file separator on linux doesn't work, we need to use the linux separator. Windows accepts either forward or backslash as file separators, so it works there.

Constantin will probably have some thoughts on this. It is really not a problem that is specific to #1634. It is just a Windows platform support issue.

As I recall, when converting on Windows, we force the file separator to be "\" in name_map.xml, right? So, when SourceNameMapper.convertName is called, the name in the 4GL source code will have "/" and the name_map.xml will have "\" - thus, it will not be able to resolve it.

What I think we need to do is to keep the program names in name_map.xml with the linux-style separator. And I think this should solve the library name problems too.

Eugenie: can you back out the ControlFlowOps hack and convert using "/" as file-separator in p2j.cfg.xml? This should procedure a name_map.xml with "/" as separator.

What I think we need to do is to keep the program names in name_map.xml with the linux-style separator.

After changing to Linux file separator the issue with unable to find the procedure is gone.

And I think this should solve the library name problems too.

But this one still here. The system still transforms "\\" string to "~\" before loading library. And in Windows this produces error.

Eugenie Lyzenko wrote:

What I think we need to do is to keep the program names in name_map.xml with the linux-style separator.

After changing to Linux file separator the issue with unable to find the procedure is gone.

OK, use this for your testing. But, if we let linux-style separator in name_map.xml, I think we still need to fix the case when we have a windows-style separator in 4GL code.

And I think this should solve the library name problems too.

But this one still here. The system still transforms "\\" string to "~\" before loading library. And in Windows this produces error.

Please post a snippet of a method-mapping node for a procedure which is mapped to a native library (i.e. there is a libname node).

Eugenie Lyzenko wrote:

This is just fragment, the others - inside attached
[...]

The ~\ comes from the 4GL code, procedure copy_ivalues external "library_calls~\testapi~\testapi.dll". Considering these comments in LibraryManager.obtainLibrary:

Parameters:
libname The library name to be loaded. In general, no modifications to this string will be done (the exception is in the native layer for Windows loading, where forward slashes are converted to backslashes).

I think the bug is in the load_library_native from library_win.c. But this is valid only if the string in name_map.xml should have been without the ~ escape char.

Greg, please advise: is this a conversion bug in the library name conversion, or a problem in load_library_native, which needs to handle the 4GL ~ escape char, too?

Greg, please advise: is this a conversion bug in the library name conversion, or a problem in load_library_native, which needs to handle the 4GL ~ escape char, too?

No.

This seems like a conversion bug. The ~\ should have been converted to \\ during conversion. Normally this is handled by using ExpressionConversionWorker.progressToJavaString(). But in this case the text is stored into the name_map.xml during annotations/collect_names.rules and the string is stored directly instead of being converted first. So this should be an easy fix to collect_names.rules.

What I think we need to do is to keep the program names in name_map.xml with the linux-style separator. And I think this should solve the library name problems too.

Yes, we need to have everything in name_map.xml be using a consistent file sep (Linux-style).

Then on input to any of the SourceNameMapper lookups, we should convert it to Linux-style before trying to do any comparisons/lookups.

Please fix these issues under Constantin's direction.

... But in this case the text is stored into the name_map.xml during annotations/collect_names.rules and the string is stored directly instead of being converted first. So this should be an easy fix to collect_names.rules.

OK. The place to fix is:

...
                  <action>
                     libname = this.getImmediateChild(prog.kw_proc, null)
                                   .getImmediateChild(prog.kw_extern, null)
                                   .getFirstChild()
                                   .text)
                  </action>
                  <action>libname = removeQuotes(libname)</action>
...

Then on input to any of the SourceNameMapper lookups, we should convert it to Linux-style before trying to do any comparisons/lookups.

Please fix these issues under Constantin's direction.

OK.

The place to fix is:

Yes. Instead of the call to removeQuotes(), it should call ExpressionConversionWorker.progressToJavaString().

The strange thing I'm struggling with. Using libname = ExpressionConversionWorker.progressToJavaString(libname) causes the exception:

...
     [java] .\library_calls\input\input_double_parameter_into_int64_is_junk.p
     [java] Elapsed job time:  00:00:05.351
     [java] ERROR:
     [java] java.lang.RuntimeException: ERROR!  Active Rule:
     [java] -----------------------
     [java]       RULE REPORT
     [java] -----------------------
     [java] Rule Type :   WALK
     [java] Source AST:  [ procedure ] BLOCK/PROCEDURE/ @0:0 {17179869623}
     [java] Copy AST  :  [ procedure ] BLOCK/PROCEDURE/ @0:0 {17179869623}
     [java] Condition :  libname = ExpressionConversionWorker.progressToJavaString(libname)
     [java] Loop      :  false
     [java] --- END RULE REPORT ---
     [java]
     [java]
     [java]
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:863)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.processTrees(ConversionDriver.java:931)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.back(ConversionDriver.java:819)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1756)
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression error [libname = ExpressionConversionWorker.progressToJavaString(libname)] [PROCEDURE id=17179869623]
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:226)
     [java]     at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:160)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.apply(PatternEngine.java:1350)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1248)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1196)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:832)
     [java]     ... 3 more
     [java] Caused by: com.goldencode.expr.ExpressionException: Expression error [libname = ExpressionConversionWorker.progressToJavaString(libname)]
     [java]     at com.goldencode.expr.Expression.getCompiledInstance(Expression.java:631)
     [java]     at com.goldencode.expr.Expression.execute(Expression.java:330)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:401)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
     [java]     at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
     [java]     at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
     [java]     at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
     [java]     at com.goldencode.
...

Using libname = ExpressionConversionWorker.progressToJavaString(libname) causes the exception:

Please search the rules files for progressToJavaString to see how to use this. The problem is simple: you cannot call it as a static method from TRPL. I just posted it that way to make it clear which TRPL worker provides that functionality.

The name_map.xml is the part of the resulting package and the place from where the application takes the exact name of the library to load. Correct?

Yes.

The name_map.xml is generating during conversion and if it contains libname entry as library_calls~\testapi~\testapi.dll - this string will be passed to the native library loader and this is the exact path for DLL to be load. Correct?

Yes. It is created during conversion and loaded from the application jar. The resulting strings must already be properly converted Java strings and cannot use any of the Progress 4GL escape sequences. progressToJavaString will solve this.

The changes for you to review to convert/compile/run testcases in Windows. The result - there are no messages during test suite run:

collect_names.rules:

...
   <worker class="com.goldencode.p2j.convert.ExpressionConversionWorker" namespace="ecw" />
...
                  <rule>libname != null
                     <action>
                        libname = ecw.progressToJavaString(libname)
                     </action>
                  </rule>
...

com/goldencode/p2j/util/character.java:

...
   public static String progressToJavaString(String progress, boolean windows)
...
            case '~':
            case '\\':
               // there must always be a following character
               i++;
               next = readChar(contents, i);

               // For Windows-target we have to make special processing for
               // '\' characters in path related strings
               if (windows && next != 'E' &&
                   next != 'b' && next != 't'  && next != 'n' && next != 'f' &&
                   next != 'r' && next != '\"' && !(next >= '0' && next <= '7'))
               {
                  if(current == '\\')
                  {
                     sb.append("\\\\");
                     if (next != '\\')
                     {
                        sb.append(next);
                     }
                     break;
                  }
                  else // current == '~'
                  {
                     if (next == '\\' || next == '/')
                     {
                        sb.append("\\\\");
                        break;
                     }
                  }
...

Also the p2j.cfg.xml file should have the following Windows specific settings:

...
      <parameter name="path-separator"   value=";" />
      <parameter name="file-separator"   value="/" />
      <parameter name="case-sensitive"   value="false" />
      <parameter name="propath"          value=".;" />
      <parameter name="basepath"         value="." />
      <parameter name="unix-escapes"     value="false" />
...

Continue working with test results.

The question I have is why we simple replace

libname = removeQuotes(libname)

libname = ecw.progressToJavaString(libname)

libname = ecw.progressToJavaString(removeQuotes(libname))

The quotes removing is not necessary?

Please post a real update so that I can use diffing tools (like meld) to compare your changes. The changes you have posted for character.java look incomplete/incorrect and I need to see the real changes to know for sure.

The quotes removing is not necessary?

At the top of progressToJavaString(), it already removes quotes. In fact, if we remove them first, then progressToJavaString() would be broken. Please read that method carefully, especially since you are editing it!

Code Review 0305b

The change to these values is concerning:

      <parameter name="file-separator"   value="/" />
      <parameter name="case-sensitive"   value="false" />

If we have to encode a UNIX file separator and case-insensitivity for Windows to work in this case then I know that there are other failures that this change will cause. In other words: these are not valid settings for Windows. For example, I know that the preprocessor will break for applications that use include file names that are in a different case than the actual filename in the file system.

The problem solved by this change must be solved in the conversion itself.

Code Review 0305a

The change to collect_names.rules is fine. The null check is not really needed, because if it is ever null, it will fail on the putAttribute call. In fact, it would be easier to debug if this failed in progressToJavaString, so I prefer to remove the null check.

The change to progressToJavaString looks incorrect. It is also not clear to me what problem this is meant to solve. Please create a testcase that has a full range of strings to test the different paths through this part of the case statement. The code can look like this:

def stream outfile.
output stream outfile to "string_cvt.log".

put stream outfile "~\".
put stream outfile "\\".
/* MANY MORE TESTS CREATED BY EVL GO HERE */

output stream outfile close.

If you try this, you will find that the 4GL DOES NOT make the kinds of changes to strings that your code would cause. We cannot put this kind of path modification code into generic string processing. Please run the tests to understand the impact. I think you have just chosen the wrong place to put the changes.

 <parameter name="file-separator"   value="/" />

This change was made according to note #96 above.

 <parameter name="case-sensitive"   value="false" />

And this one is not related to current task, so can easily be undone.

Please create a testcase that has a full range of strings to test the different paths through this part of the case statement.

OK.

This change was made according to note #96 above.

I understand. But my concern is that changing the p2j.cfg.xml has other implications that may make this a bad idea. I think we need to more explicitly cleanup the paths during the creation of the name_map.xml itself. Then there is no reason to change the p2j.cfg.xml (which can break other code) and there is no reason to put nasty hacks in ControlFlowOps.

And this one is not related to current task, so can easily be undone.

What were you doing that made this useful? Does removing it cause some other problem?

I understand. But my concern is that changing the p2j.cfg.xml has other implications that may make this a bad idea. I think we need to more explicitly cleanup the paths during the creation of the name_map.xml itself. Then there is no reason to change the p2j.cfg.xml (which can break other code) and there is no reason to put nasty hacks in ControlFlowOps.

Agree. In Windows p2j.cfg.xml we need to keep the native file separator '\'. At least because the P2J java call to request file separator from JVM will always return '\' in Windows so we need to be in sync here.

What were you doing that made this useful? Does removing it cause some other problem?

I'm using case-sensitive==false in Windows conversions because the Windows file system is really case insensitive as I've found before. It is possible to use different cases like File.txt and fILe.txt but really it will be the same file. The only problem that can be is to convert files like progress_file.p, progress-file.p and progressFile.p. The result is the same ProgressFile.java class.

But for the current task this is not important, so this change was just inherited from other Windows conversion cases.

No, you are right. The p2j.cfg.xml.win should have case-sensitive==false. Keep this change.

I understand. But my concern is that changing the p2j.cfg.xml has other implications that may make this a bad idea. I think we need to more explicitly cleanup the paths during the creation of the name_map.xml itself.

Let me clarify one point. Assume:
1. We have Windows file separator '\' in conversion config file
2. Linux style source file to convert in Windows containing: run library_calls/load_unload/load_unload_tests.p.

The questions:
The way A.
1. The respective name_map.xml entry should contain library_calls/load_unload/load_unload_tests.p. with Linux style file separator?
2. In this case the converted Java code will have ControlFlowOps.invoke("library_calls/load_unload/load_unload_tests.p"); code.

This is expected as the final result?

Or another way(B):
1. The name_map.xml entry will be transformed to: library_calls\\load_unload\\load_unload_tests.p. replacing file separator.
2. The converted Java code is also transforming to ControlFlowOps.invoke("library_calls\\load_unload\\load_unload_tests.p"); code.

I guess we need to choose A or B to properly map names to objects. And this is not a file system point because when the name resolution is happening there are no actual request to the file system so technically these ways are equivalent. Correct? I'm not sure what is the best way for Windows conversion, inclining to way B just because this looks more "native" for Windows code. The way A has another good point - no need to modify names in converted java code. What do you think?

We want to implement all name_map.xml entries in Linux form, even if the app was written with Windows file seps. In other words, the name_map.xml entries should all be in a "regular" form, which is the Linux form.

Thinking about the runtime implications, I am considering that the converted code may be calculating character expressions at runtime that have Windows file seps. This means that our ControlFlowOps code OR possibly our SourceNameMapper code must convert these runtime names into the "regular" form (Linux file seps) BEFORE doing lookups. I think this has to be done in our runtime layer and cannot be done at conversion time.

Constantin: please provide your thoughts on where exactly to put this code. I know we need to retain the actual string that is passed in for certain messages and attribute responses. So deciding on a location to call this code is tricky. Hopefully, the code can be written in a single helper method that can be called from wherever necessary (including at conversion time for the entries in the name_map.xml).

Greg Shah wrote:

Constantin: please provide your thoughts on where exactly to put this code. I know we need to retain the actual string that is passed in for certain messages and attribute responses. So deciding on a location to call this code is tricky. Hopefully, the code can be written in a single helper method that can be called from wherever necessary (including at conversion time for the entries in the name_map.xml).

The legacy-program-name to ExternalProgram mappings are kept in the SourceNameMapper.p2j map; thus, this map will always be populated with names using the standard linux-style separators, as they are read from name_map.xml. Before interogating this map, the received legacy name must be processed, so that all the windows-style separators are replaced with linux-style separators.

The good part is that all p2j.get references in SourceNameMapper are of the p2j.get(canonicalize(progName)) form. Thus, SourceNameMapper.canonicalize should be changed so that the window-style separators in the received program name are processed, before canonicalizing the program name.

We want to implement all name_map.xml entries in Linux form, even if the app was written with Windows file seps. In other words, the name_map.xml entries should all be in a "regular" form, which is the Linux form.

OK. And what about native library path names? For example "library_calls/testapi/testapi.dll". Here we also need Linux form of file sep in name_map.xml?

And what about native library path names? For example "library_calls/testapi/testapi.dll". Here we also need Linux form of file sep in name_map.xml?

No. Do not make any changes to the library names, except for what you are already doing with progressToJavaString. In almost all cases, the 4GL just passes these strings unchanged to the native OS and we will do the same thing. There is 1 exception on Windows, but we already handle it in our native layer for library loading.

Some finding for execution the test in 4GL systems:

Source:

def stream outfile.
output stream outfile to "string_cvt.log".

put stream outfile "~\" skip.
put stream outfile "\\" skip.
/* MANY MORE TESTS CREATED BY EVL GO HERE */
put stream outfile "~/" skip.
put stream outfile "\\\\" skip.
put stream outfile "Alternative\path" skip.
put stream outfile "Alternative/path" skip.

output stream outfile close.

Windows output:

\
\\
/
\\\\
Alternative\path
Alternative/path

Linux output:

\
\
/
\\
Alternativepath
Alternative/path

Code Review 0306a/b

I am fine with the changes.

Questions:

1. With this change, does the Windows native API support work properly?

2. Are there any diffs in the converted code caused by this change (other than name_map.xml)?

1. With this change, does the Windows native API support work properly?

I think yes. There are some diffs regarding to math constants on output file, in Linux we have 1.234e+18, while in Windows - 1.234e+018 (3 digits in exponent field) and nan, -nan values when we have 1.#QNAN, -1.QNAN or SNAN

2. Are there any diffs in the converted code caused by this change (other than name_map.xml)?

Yes, there are the diffs in java converted code in load_unload tests and one from misc test. Some of them are related to native library name difference(and this is expected) but there are set of diffs that are not obvious and I need to find out if they are OS dependent and expected or not.

2. Are there any diffs in the converted code caused by this change (other than name_map.xml)?

After check I confirm all java generated code diffs are expected and related to usage

&if "{&OPSYS}" eq "win32" &then
...
&else
...
&endif

Excellent! OK, please go ahead with the following:

1. Check in your 0306b change.

2. Do a conversion regression test of 0306a. There should be NO changes to any of the files including the name_map.xml. Please document the results here.

1. Check in your 0306b change.

Committed in bzr as 1116.

2. Do a conversion regression test of 0306a. There should be NO changes to any of the files including the name_map.xml. Please document the results here.

OK.

The conversion testing finished. There are no differences in generated java code in src/aero/timco/majic including name_map.xml file for original conversion and after 0306a fix.

I think runtime regression is not required because no P2J runtime code was involved in this fix.

Agreed. You can check in and distribute your change.

Greg Shah wrote:

Agreed. You can check in and distribute your change.

I think we forgot about a case: RUN <window-style-path> needs to have the path processed at runtime...

Where is the least-intrusive place to process that? It cannot be done at conversion time since it can be a runtime-generated name.

Eugenie: you can still commit your changes and distribute. We just can't close this task until you fix this other issue.

Greg Shah wrote:

Where is the least-intrusive place to process that? It cannot be done at conversion time since it can be a runtime-generated name.

I've explained this in note 117 - SourceNameMapper.canonicalize should be changed so that the window-style separators in the received program name are processed, before canonicalizing the program name.

Sorry, I should have picked up on that.

Eugenie: your next step is to implement proper support in SourceNameMapper.canonicalize as described in note 117.

Eugenie: your next step is to implement proper support in SourceNameMapper.canonicalize as described in note 117.

OK.

The changes to fix name_map.xml generation has been committed in bzr as 10491.

Don't forget to send out the email too.

Which cases we need to handle for Windows:

1. run path\to\program.p
2. run path\\to\\program.p
3. run path~\to~\program.p

Or all of them? I'll check the behavior on WINDEV01 but now it is not available to use.

I don't think that case 1 will work in the 4GL on Windows. The other two definitely need to be supported.

Also: make sure you have tests including:

• drive letter + COLON + relative paths
• drive letter + COLON + absolute paths
• no drive + relative paths
• no drive + absolute paths
• variants of the above with ../ portions included
• no path at all (just the base filename itself)

Also: make sure you have tests including:

drive letter + COLON + relative paths
drive letter + COLON + absolute paths
no drive + relative paths
no drive + absolute paths
variants of the above with ../ portions included
no path at all (just the base filename itself)

OK.

I have tested the following in WINDEV01:

1. run path\to\program.p
2. run path\\to\\program.p
3. run path~\to~\program.p
4. run path/to/program.p
5. run path~/to~/program.p

I've been surprised but all 1-5 cases work fine in Windows.

1. run path\to\program.p
2. run path\\to\\program.p
3. run path~\to~\program.p
4. run path/to/program.p
5. run path~/to~/program.p

The first point I'm thinking about is how to convert these cases. My suggestion is the converted code(meaning runtime level) should have only one interpretation for Windows file separator - Java compatible for Windows("\\"). All other cases must be transformed to "\\". What do you think about this approach?

The point number two is to find the right place for modification. Working on this.

And another question. How the code above be converted/run in Linux? Transform all cases to "/"? Or we will not convert such code in Linux?

Eugenie Lyzenko wrote:

The first point I'm thinking about is how to convert these cases. My suggestion is the converted code(meaning runtime level) should have only one interpretation for Windows file separator - Java compatible for Windows("\\"). All other cases must be transformed to "\\". What do you think about this approach?

You need to keep in mind that these paths eventually end up resolved by SourceNameMapper, which matches it with names in name_map.xml. As the names in name_map.xml are using linux-style separator, the windows-style separators in the path need to be transformed to linux-style separators. I suspect you may need to call ExpressionConversionWorker.progressToJavaString, to make it a valid java string, before replacing the window-style separator to linux-style separator.

The point number two is to find the right place for modification. Working on this.

Per my notes in 117, I think the best place to do this is SourceNameMapper.canonicalize - if you find this is not working, please post a testcase showing that.

And another question. How the code above be converted/run in Linux?

This is a good question. We need to decide if we should allow window-style code to be executed on a linux machine (by "tricking" the P2J runtime to think the OPSYS is windows) and viceversa; if OS-dependent native libraries or processes are not invoked, then the code can be executed safely. Another problem is that the \ char is a valid char in a linux-style filename... I know, this is exotic, but this means that we can't 100% replace all \ with /, regardless of the OS on which P2J is ran. Thus I think is best to limit the replacement of \ with / only for Windows OS.

Greg, on a side note: runtime may decide to pass an absolute path to a RUN statement; IIRC, this works both on windows and linux. We can postpone this for a future task, as I don't think server project uses such cases; also, in my opinion, this looks like poor design in the 4GL code and the fix should be there, not in our P2J runtime/conversion rules.

My suggestion is the converted code(meaning runtime level) should have only one interpretation for Windows file separator - Java compatible for Windows("\\"). All other cases must be transformed to "\\". What do you think about this approach?

I think we already do this in progressToJavaString, although the single \ case may be a problem for us. We need to implement the strings the same way Progress would in the case that unix-escapes is false. Otherwise, I don't see the need for any changes.

And another question. How the code above be converted/run in Linux?

This is a good question. We need to decide if we should allow window-style code to be executed on a linux machine (by "tricking" the P2J runtime to think the OPSYS is windows) and viceversa;

I don't think we need to worry about this. If someone codes things to be Windows-specific in the 4GL, then the 4GL itself would not work on Linux for that case. We have enough to do to be compatible without adding new features that are not needed.

Another problem is that the \ char is a valid char in a linux-style filename... I know, this is exotic, but this means that we can't 100% replace all \ with /, regardless of the OS on which P2J is ran. Thus I think is best to limit the replacement of \ with / only for Windows OS.

I agree. We do this replacement in 2 places:

1. name_map.xml entries
2. SourceNameMapper.canonicalize

Greg, on a side note: runtime may decide to pass an absolute path to a RUN statement; IIRC, this works both on windows and linux. We can postpone this for a future task, as I don't think server project uses such cases; also, in my opinion, this looks like poor design in the 4GL code and the fix should be there, not in our P2J runtime/conversion rules.

It is poor design, but it is something that we can handle in the runtime. I don't want to require that code changes are needed, when it is not a big deal to support the bad/poor code. There are many, many places that would qualify as poor code and we still support them all because to require all of those places to be edited before conversion is an extra cost that should be optional/recommended but not a prerequisite.

Eugenie: please do provide full support for absolute (drive letter) paths on Windows.

Eugenie: please do provide full support for absolute (drive letter) paths on Windows.

OK.

Another finding. The cases with tilde char in original 4GL code like

1. run path~\to~\program.p
2. run path~/to~/program.p

1. run path\to\program.p
2. run path/to/program.p

I think we already do this in progressToJavaString, although the single \ case may be a problem for us.

Yes, this can be a problem.

We need to implement the strings the same way Progress would in the case that unix-escapes is false. Otherwise, I don't see the need for any changes.

I see. But for run program.p (and only for this case) I would suggest the radical approach. What if we always use linux file sep '/' in such a cases for final converted java code? Is it acceptable or not?

So I guess we must transform path\to\program.p string to path\\to\\program.p for java converted code

Modify progressToJavaString to handle this case when running in windows mode. This should be safe because in this mode we know that \ cannot be used as an escape sequence, so it must have been an escaped character itself.

But for run program.p (and only for this case) I would suggest the radical approach. What if we always use linux file sep '/' in such a cases for final converted java code? Is it acceptable or not?

No, there is no need for this. We still have to make the SourceNameMapper.canonicalize() changes. That change will handle this case too.

The question: do we plan to keep predefined "/" file separator value in directory.xml file?

It is not "predefined". That directory.xml is just a file that was designed to be run on Linux. All the values in there are configuration values. When you install a new P2J server (or move one), you must make sure the configuration values are correct.

Code Review 0311a

1. The change in progressToJavaString() will cause problems with strings in Windows mode where the next char after the \ is a quote char or a tilde. For example:

'path\to\''something'
"path\to\""something"
"path\to\~~tsomething"

If I recall properly, the double tilde will leave a tilde behind from the preprocessor. In the last example, it would turn into a tab char in progressToJavaString().

Instead of unconditionally appending the next char (when it ISN'T \), this code just needs to BYPASS (i.e. NOT append) the next char if it IS \. That way in all other cases, the following char is processed as normal and can get special processing when needed for tilde or quote.

2. There must always only be ONE history entry for your work. Please don't put multiple entries in for a single task. character.java has this problem.

3. The history entry for control_flow.rules is not descriptive enough. It is too cryptic right now. Please describe what the exact problem is and what is being done to solve it.

4. In SourceNameMapper, the approach is not right. As you note, there are cases it doesn't handle. Why can't we do something very simple: if the fileSep is '\\', then just use String.replaceAll() to convert all '\\' characters with '/' BEFORE we use String.split()? We can't always do this, because on Linux, the \\ is used as an escape char.

5. It seems that we need changes in the other SourceNameMapper code using fileSep, since these will not be right either. The idea now is that all of our internal data must be using / instead of \\, so we must initialize all our data structures with "converted" values.

6. I don't see any changes relating to Windows files names that have a drive letter and colon. Are drive letters already supported for both absolute and relative names?

It is not "predefined". That directory.xml is just a file that was designed to be run on Linux. All the values in there are configuration values. When you install a new P2J server (or move one), you must make sure the configuration values are correct.

Does it mean all file-system container values in directory.xml file for the OS where the server is running should contain proper values for given OS and P2J code can trust the propath, path-separator and case-sensitive will correspond to the OS?

2. There must always only be ONE history entry for your work. Please don't put multiple entries in for a single task. character.java has this problem.

OK.

5. It seems that we need changes in the other SourceNameMapper code using fileSep, since these will not be right either. The idea now is that all of our internal data must be using / instead of \\, so we must initialize all our data structures with "converted" values.

Another word the fileSep inside SourceNameMapper should always be "/" I guess.

6. I don't see any changes relating to Windows files names that have a drive letter and colon. Are drive letters already supported for both absolute and relative names?

After short testing I've found these cases does not work with 0311a and need more changes. Working on this.

Does it mean all file-system container values in directory.xml file for the OS where the server is running should contain proper values for given OS and P2J code can trust the propath, path-separator and case-sensitive will correspond to the OS?

These values must be set based on the ORIGINAL system on which the 4GL code was running.

For example: if the 4GL code was running on Windows, then these values would be configured for Windows (fileSep \, case-sensitive false...).

These values have nothing to do with the system on which the current P2J client is running and even less to do with where the P2J server is running.

1. In your example is it key point the string leading to be the same as characters inside the strings(both ' or ")?

Yes. This is a common way to "escape" the quote character being used for the string, like this:

'This is a single embedded '' quote character'

This is NOT 2 strings, but a single string with an escaped ' inside.

For Windows the expected final path strings for converted code are:

Here I am not so sure. It depends on what Progress does in this same case. We need to do the following:

1. Generate the same string.
2. Make sure it can compile in Java.

Another important thing to clarify.

Observing the full deployment path from conversion the source to run the converted code I consider the following, correct me if I'm wrong:

1. On the conversion stage the driver map the 4GL file name to the internal class for every program to be converted that can be found based on the PROPATH setting known on the conversion stage. The source is converted only the file(with possible path part) can be found in one of the PROPATH directories. The name of the file to put into name_map.xml is(or should be) "normalized" to have the partial path name that can be located in one of the PROPATH directories. The corresponding class name is the file name having "/" replaced with "." and properly modified name. So we should have exact one-one mapping between 4GL source and converted class.

2. The name_map.xml is used as helper file to share mapping info between conversion and runtime.

3. When converted code is started it can have PROPATH value different than the conversion stage. But usual it should be the same to avoid mapping issues. The invoke() call(converted RUN statement) to execute the program operates with the original path name as it was in 4GL source(with file sep handling of course). To find the respective class server should canonicalize the file name to the form that is referred to one of the PROPATH directory without the PROPATH element itself. Then the java class is taken by the server from the map that is built using name_map.xml file. If class can be located - it is started to run.

Everything is correct in this understanding?

What if the 4GL source file(absolute or relative) is not in the PROPATH? It will be non converted with conversion exception error?

What if we have PROPATH like this: "C:\SomePath;D:\SomePath" with only drive letter difference? It is not possible to have the same name files in both directories because the base java package for converted classes will be the same and the java class name does not have any info about full 4GL source path?

On the conversion stage the driver map the 4GL file name to the internal class for every program to be converted

Yes

that can be found based on the PROPATH setting known on the conversion stage. The source is converted only the file(with possible path part) can be found in one of the PROPATH directories.

No

We give the conversion a list of files to process. The PROPATH is only used to find include files. It has nothing to do with the external procedures.

The name of the file to put into name_map.xml is(or should be) "normalized"

Yes.

that can be located in one of the PROPATH directories.

No. The normalized form is based on the "project home".

The corresponding class name is the file name having "/" replaced with "." and properly modified name. So we should have exact one-one mapping between 4GL source and converted class.

Yes (although by 4GL source I would mean external procedure).

2. The name_map.xml is used as helper file to share mapping info between conversion and runtime.

Yes.

3. When converted code is started it can have PROPATH value different than the conversion stage. But usual it should be the same to avoid mapping issues. The invoke() call(converted RUN statement) to execute the program operates with the original path name as it was in 4GL source(with file sep handling of course). To find the respective class server should canonicalize the file name to the form that is referred to one of the PROPATH directory without the PROPATH element itself. Then the java class is taken by the server from the map that is built using name_map.xml file. If class can be located - it is started to run.

Yes.

What if the 4GL source file(absolute or relative) is not in the PROPATH? It will be non converted with conversion exception error?

PROPATH only matters for include files at conversion time.

What if we have PROPATH like this: "C:\SomePath;D:\SomePath" with only drive letter difference? It is not possible to have the same name files in both directories because the base java package for converted classes will be the same and the java class name does not have any info about full 4GL source path?

All files to be converted must reside within a single subdirectory under the project home. This ensures that we can create a package structure in the jar file that is valid.

The PROPATH that is defined for conversion and runtime can be different from the original 4GL PROPATH (and different from each other since they are used for different purposes).

All files to be converted must reside within a single subdirectory under the project home. This ensures that we can create a package structure in the jar file that is valid.

The PROPATH that is defined for conversion and runtime can be different from the original 4GL PROPATH (and different from each other since they are used for different purposes).

And after the conversion has been completed it is not required to keep the original 4GL source tree on the server system because the runtime code in java has nothing to do with *.p files? Correct?

Another word we do not need the PROPATH on the runtime stage at all? But the "project home" value is required to be able to normalize the invoke("path/to/program.p") absolute or relative converted program name string to the form "project/home/path"+"path/located/in/name_map_xml/program.p".

I'm asking because the SourceNameMapper.canonicalize() is used in different places inside SourceNameMapper and it should return program path related to the project home.

And after the conversion has been completed it is not required to keep the original 4GL source tree on the server system because the runtime code in java has nothing to do with *.p files? Correct?

Yes.

Another word we do not need the PROPATH on the runtime stage at all?

Incorrect. We DO need the PROPATH at runtime. This is how filenames specified in RUN statements get found.

But the "project home" value is required to be able to normalize the invoke("path/to/program.p") absolute or relative converted program name string to the form "project/home/path"+"path/located/in/name_map_xml/program.p".

Yes.

I'm asking because the SourceNameMapper.canonicalize() is used in different places inside SourceNameMapper and it should return program path related to the project home.

I don't think that canonicalize() has this job.

Constantin?

Greg Shah wrote:

I'm asking because the SourceNameMapper.canonicalize() is used in different places inside SourceNameMapper and it should return program path related to the project home.

I don't think that canonicalize() has this job.

canonicalize() will prepare the name so that it can be matched against a name from name_map.xml. Also, all names in name_map.xml are emitted such as they are relative to the configured basepath, not project home. The fact that basepath is the same as project home for the testcases/uast project is just a coincidence.

Eugenie: is your problem about absolute names in RUN statements? Currently, it is assumed that all program names given to RUN statements can be resolved using the runtime propath, but the propath is again just a list of folders from the basepath. If the 4GL code passes absolute names to RUN statements (i.e. starting with a drive-letter prefix on Windows) we could configure a list of absolute names, so that we can remove the absolute prefix and make it basepath-relative. Alternatively, we can let this be done by a customer-specific XML mapping of filenames outside of propath, so that they can be resolved at runtime. Another choice would be to force the customer to fix the 4GL code so that all names can be resolved properly from the runtime propath, and avoid all this messiness in P2J runtime.

canonicalize() will prepare the name so that it can be matched against a name from name_map.xml. Also, all names in name_map.xml are emitted such as they are relative to the configured basepath, not project home.

I need to know what exactly the "configured basepath" variable is on the run-time stage. Where it is defined in config file and how it is known by SourceNameMapper class at run time.

Eugenie: is your problem about absolute names in RUN statements?

Actually no. This case works with properly configured searchpath value in directory.xml file. The problem is in execution relative path like(the uast is the current directory to start the client):

run rundir\..\..\uast\rundir\relative\test11.p
run ..\uast\rundir\relative\test5.p

The other issue is the canonicalize() is used not only in convertName() call. So I need to know what exact canonicalize() should do and what should do not. If canonicalize() return value is the name_map.xml compatible program path - it should remove absolute and relative path elements. In this case we will perform this work twice, once in canonicalize() and the other code already exists in convertName() call. So what are the expected result for:

canonicalize("c:/path/to/testcases/uast/rundir/absolute/test4.p")
canonicalize("../uast/rundir/relative/test5.p")

Eugenie Lyzenko wrote:

I need to know what exactly the "configured basepath" variable is on the run-time stage. Where it is defined in config file and how it is known by SourceNameMapper class at run time.

SourceNameMapper.proroot (the basepath) is used only at conversion time. At runtime is not used at all - and I don't think we would want to use it.

Actually no. This case works with properly configured searchpath value in directory.xml file.

• If yes, this approach is not OK: the P2J server should have no dependency on the physical files for the legacy sources. At runtime, always assume the legacy 4GL programs do not exist on disk - all you have is the converted code.
• Else, please give an example.

rundir\..\..\uast\rundir\relative\test11.p
../uast/rundir/relative/test5.p

Both cases contain folder names outside of the conversion-time basepath and can not be mapped using the runtime propath. This is a dependency which I think should be resolved at runtime via a customer-specific mapping. We can assume that no folder/path from the legacy system exists on the machine where the P2J server is ran, so we can't easily resolve this, without customer-specific mappings. And the runtime PROPATH can not help us either, because all it will contain are folders relative to the conversion-time basepath.

The current code to remove ".." is seems to be incorrect in canonicalize(). It removes the path entries before ".." ans as you can see the result is wrong.

Yes, it is incorrect in this case, but I think canonicalize was initially written to work only with paths from the propath.

If canonicalize() return value is the name_map.xml compatible program path - it should remove absolute and relative path elements.

I think you are correct: canonicalize() needs to be a little smarter, to check for customer-specific mappings first and after that move into the other processing, to replace all windows-style separator to unix-style separator.

So what are the expected result for:
canonicalize("c:/path/to/testcases/uast/rundir/absolute/test4.p")

This needs to be rundir/absolute/test4.p

canonicalize("../uast/rundir/relative/test5.p")

This needs to be rundir/relative/test5.p

Please double check something else: on windows, the names are case-insensitive: what if the legacy file-name was i.e. UPPERCASE.p and the run statement uses RUN uppercase.p ? Will these match on windows, with current P2J?

I don't understand: do you match an absolute name for the legacy 4GL program to an EXISTING file on the disk?

If yes, this approach is not OK: the P2J server should have no dependency on the physical files for the legacy sources. At runtime, always assume the legacy 4GL programs do not exist on disk - all you have is the converted code.
Else, please give an example.

Not exactly. For example consider:

run c:\path\to\testcases\uast\rundir\absolute\test4.p

The name_map.xml will have rundir/absolute/test4.p entry after conversion running in c:\path\to\testcases\uast. But the converted code will have:

invoke("c:\\path\\to\\testcases\\uast\\rundir\\absolute\\test4.p");

When the server is starting there is no physical dependency on the test4.p file on the disk, we do not need to work with it. But we have to work with the path string from invoke() method. And here the PROPATH variable can help. If it has the c:/path/to/testcases/uast value we are removing this absolute part from path string getting "rundir\\absolute\\test4.p" and this value can be properly mapped to the Java class to execute.

Otherwise the mapping for test4.p file name become problematic because we need to enumerate whole p2jMap object finding the class to be matched only name. And also it can brings the conflicts with classes like rundir.absolute.Test4 and rundir.relative.Test4. So I think we still need the @PROPATH variable to be known and properly configured on runtime stage.

rundir\..\..\uast\rundir\relative\test11.p
../uast/rundir/relative/test5.p

Both cases contain folder names outside of the conversion-time basepath and can not be mapped using the runtime propath. This is a dependency which I think should be resolved at runtime via a customer-specific mapping. We can assume that no folder/path from the legacy system exists on the machine where the P2J server is ran, so we can't easily resolve this, without customer-specific mappings. And the runtime PROPATH can not help us either, because all it will contain are folders relative to the conversion-time basepath.

The PROPATH can contain absolute path elements, like "c:/path/to/testcases/uast" and on runtime we can apply relative path "../uast/rundir/relative/test5.p" to the PROPATH value having "c:/path/to/testcases/uast/../uast/rundir/relative/test5.p". Then removing ".." gives "c:/path/to/testcases/uast/rundir/relative/test5.p", then removing PROPATH itself gives "rundir/relative/test5.p" and this value can be mapped by server runtime. That's how the PROPATH can work even if we do not have the physical 4GL source tree on the target system. The "rundir\..\..\uast\rundir\relative\test11.p" will give the correct mapping for "rundir/relative/test11.p" name the same way.

So I think it is up to canonicalize() to do all such work. It this OK?

Eugenie Lyzenko wrote:

When the server is starting there is no physical dependency on the test4.p file on the disk, we do not need to work with it. But we have to work with the path string from invoke() method. And here the PROPATH variable can help. If it has the c:/path/to/testcases/uast value we are removing this absolute part from path string getting "rundir\\absolute\\test4.p" and this value can be properly mapped to the Java class to execute.
...
So I think it is up to canonicalize() to do all such work. It this OK?

Great idea, this will work. Document these assumptions at the convertName javadoc (that runtime PROPATH is expected to contain such paths).

canonicalize() doesn't need to know about the propath, only the convertName needs to search it. The other SourceNameMapper APIs which call canonicalize all receive a basepath-relative name; only convertName receives the filename from the RUN statement.

• control_flow.rules - I don't understand what this code tries to do:
  

                 <!-- This is required for additional progressToJavaString() call, it needs a
                      quotes but only when the filename does not have ",' or ~ inside
                      -->
                 <rule>childref.text.indexOf('"') != -1 or
                       childref.text.indexOf("'") != -1 or
                       childref.text.indexOf('~') != -1
                    <action>legacyname = childref.text</action>
                    <action on="false">
                       legacyname = ecw.progressToJavaString(sprintf("'%s'", childref.text), true)
                    </action>
                 </rule>
  

  
  Shouldn't the filename already have been preprocessed by the progress.g's run_stmt rule, which has this code?
  

                String txt = character.progressToJavaString(#s.getText(),
                                         !Configuration.getParameter("unix-escapes", true),  true);
                #s.setText(txt);
  

  
  I don't understand why you need to call progressToJavaString twice, once in progress.g and once in control_flow.rules.
• SourceNameMapper.canonicalize - why are you using the searchPath only for windows? Isn't it possible to have absolute paths in linux, too?

I don't understand why you need to call progressToJavaString twice, once in progress.g and once in control_flow.rules.

The key point here is we need to call progressToJavaString() second time to convert both "\" and "\\" to valid file separator elements to be compiled in Java - "\\". Without this call the "\" remains "\" in converted java code and is not able to compile. But if the filename string contains special elements inside and only inside string like "" or '' or ~~ it is not necessary and will produce wrong results in Windows. Like in these examples:

run 'run\''test8.p'.
run "run\""test9.p".
run "run\~~ttest10.p".

SourceNameMapper.canonicalize - why are you using the searchPath only for windows? Isn't it possible to have absolute paths in linux, too?

Because in Linux we already do this work in convertName() method after canonicalize(). I just do not want to break the existed code making as small changes as possible. Otherwise we need to fully rework convertName() to disable double working blocks in both canonicalize() and convertName(). Note the canonicalize() is used in many other places than only convertName().

BTW. The conversion of the MAJIC code with and without the changes from this update shows identical conversion results in Linux.

Eugenie Lyzenko wrote:

Like in these examples:

For these examples, and the ones on note 161, please post how the converted result needs to look on windows and how the physical filenames looks on the disk.

SourceNameMapper.canonicalize - why are you using the searchPath only for windows? Isn't it possible to have absolute paths in linux, too?

Because in Linux we already do this work in convertName() method after canonicalize(). I just do not want to break the existed code making as small changes as possible. Otherwise we need to fully rework convertName() to disable double working blocks in both canonicalize() and convertName(). Note the canonicalize() is used in many other places than only convertName().

OK, I understand your approach. But I think we can make it more compact: only SourceNameMapper.convertName can receive a name which is not an unique name found in name-map.xml. All other APIs in SourceNameMapper, although at this time they call SourceNameMapper.canonicalize, they should receive only an unique name from name-map.xml. Thus, we can merge the logic from canonicalize into convertName.

For these examples, and the ones on note 161, please post how the converted result needs to look on windows and how the physical filenames looks on the disk.

Not always it is possible to have the filenames from this list. The filename "test.p is not valid one for Windows OS(the filename can not have " character). But for other files the valid names are: 'test8.p and ~ttest10.p the other names are obvious. The both "\" and "\\" file separators are considered as the same "\\"(in java string perspective). Also both "/" and "~/" are also the same "/".

But this is not the same as output for testcase:

output stream outfile to "string_cvt.log".
put stream outfile "~\" skip.
put stream outfile "\\" skip.
put stream outfile "~/" skip.
put stream outfile "\\\\" skip.
put stream outfile "Alternative\path" skip.
put stream outfile "Alternative/path" skip.
put stream outfile 'path\to\''something' skip.
put stream outfile "path\to\""something" skip.
put stream outfile "path\to\~~tsomething" skip.
output stream outfile close.

In Windows we have:

\
\\
/
\\\\
Alternative\path
Alternative/path
path\to\'something
path\to\"something
path\to\~tsomething

This is a bit different because the 4GL thinks these are not a filename but regular strings. And we need to support both reactions in Windows I think. That's why I have introduced one more option in progressToJavaString() method.

OK, I understand your approach. But I think we can make it more compact: only SourceNameMapper.convertName can receive a name which is not an unique name found in name-map.xml. All other APIs in SourceNameMapper, although at this time they call SourceNameMapper.canonicalize, they should receive only an unique name from name-map.xml. Thus, we can merge the logic from canonicalize into convertName.

OK. I'll work on this. Another word when the canonicalize() is calling not from convertName() the name can not contain the substrings like ".." or "." or "". Correct? And we do not need the loop to remove them inside canonicalize()?

Thus, we can merge the logic from canonicalize into convertName.

And what in this plan canonicalize() should do? Just replace incoming fileSep with "/"?

Code Review 0320a

I will let Constantin review and comment on the core logic changes in SourceNameMapper.

1. In SourceNameMapper.convertName(), I think there may be a problem with the usage of PlatformHelper.isUnderWindowsFamily(). This is running on the SERVER, so it is checking if the server OS is Windows. But what we care about is whether the CLIENT is running on Windows or not. All these paths and filenames are client-side names.

2. In character.progressToJavaString(), shouldn't this code:

if (isPathName && i < end && readChar(contents, i+1) == '\\')

be this:

int j = i + 1;

if (isPathName && (j < end && readChar(contents, j) == '\\')

2. In character.progressToJavaString(), shouldn't this code:

 if (isPathName && i < end && readChar(contents, i+1) == '\\')
 

be this:

 int j = i + 1;

 if (isPathName && (j < end && readChar(contents, j) == '\\')
 

Yes, you are right. The code assumed there is at least one char after "\". But we need to handle the case when it is not a true. So we could use (i+1 < end) instead of introducing the new variable but the code you offered is more clear to read and I'll change the character.java in next drop.

Greg Shah wrote:

1. In SourceNameMapper.convertName(), I think there may be a problem with the usage of PlatformHelper.isUnderWindowsFamily(). This is running on the SERVER, so it is checking if the server OS is Windows. But what we care about is whether the CLIENT is running on Windows or not. All these paths and filenames are client-side names.

Theoretically, until native processes are involved, P2J has no restrictions for the OS on which the client or the server is ran, and this suggests that the reference would be the server's OS. But your concern I think is valid: there might be cases when an application is configured to run from Windows for some clients and from Linux for others... each of them accessing different parts of the application, using OS-specific parts. So checking the client's OS looks correct.

• canonicalize needs to be called only from convertName, as the other callers all need to receive an unique filename, as it appears in name_map.xml. Please check the SourceNameMapper API usage (except convertName and java-to-progress APIs), if the legacy external procedure name passed to the API was first made "absolute" via a ProcedureManager.getAbsoluteName call.
• change the javadoc for the fileSep field to indicate that this separator is always used in name_map.xml, regardless of OS.

Eugenie Lyzenko wrote:

I do not understand why we need the changes here? If canonicalize() now works as before any modifications why do we need to modify the logic?

When I changed the FILE-NAME and NAME implementation for external procedures, I discovered that these attributes and the error messages containing the ext prog name need to use the name as passed to the RUN statement. For this reason, the SourceNameMapper APIs (beside convertName) need to receive a name_map.xml "absolute" name as the argument for a legacy external procedure name, and not the name passed at the RUN statement. So, calling canonicalize in other APIs beside convertName is a no-op, and can confuse readers. I should have done these changes when I added the FILE-NAME changes, but I missed it, so please change it now.

Done. But however the SourceNameMapper.overwriteMappingData() still can change the fileSep value.

OK, then change it to a "At runtime, this is OS independent value used internally...".

When I changed the FILE-NAME and NAME implementation for external procedures, I discovered that these attributes and the error messages containing the ext prog name need to use the name as passed to the RUN statement. For this reason, the SourceNameMapper APIs (beside convertName) need to receive a name_map.xml "absolute" name as the argument for a legacy external procedure name, and not the name passed at the RUN statement. So, calling canonicalize in other APIs beside convertName is a no-op, and can confuse readers. I should have done these changes when I added the FILE-NAME changes, but I missed it, so please change it now.

So for example instead of having in SourceNameMapper:

...
   public static String getMethodName(String  pname, 
                                      String  ieName, 
                                      boolean function)
   {
      initMappingData();
      ExternalProgram extProg = p2jMap.get(canonicalize(pname));
...

...
   public static String getMethodName(String  pname, 
                                      String  ieName, 
                                      boolean function)
   {
      initMappingData();
      ExternalProgram extProg = p2jMap.get(ProcedureManager.getAbsoluteName(pname));
...

Eugenie Lyzenko wrote:

Or we do not need any additional processing, just call p2jMap.get(pname);?

This one is correct, no additional processing is required.

Eugenie, the changes in 0324a.zip look good.

Eugenie, the changes in 0324a.zip look good.

Meaning I can run conversion and runtime regression testing for this change? BTW my local system conversion shows no difference for generated java code for with and without this change.

There is still an issue in SourceNameMapper.convertName() with the usage of PlatformHelper.isUnderWindowsFamily(). This needs to be a client-side check.

There is still an issue in SourceNameMapper.convertName() with the usage of PlatformHelper.isUnderWindowsFamily(). This needs to be a client-side check.

Yes, I've missed this call. There are two possible approaches to this fix:
1. We can ignore OS dependency and call "\" substitutions unconditionally.
2. Or we can insert call to LogicalTerminal->ThinClient->PlatformHelper to the initMappingData() to set up the local boolean member, say clientOsIsWinodws = LogicalTerminal.isUnderWindowsFamily(); to not to call client side every time we call convertName().

What do you think? Looks like #2 is better?

No, we need to fix EnvironmentOps.getOperatingSystem() and EnvironmentOps.getPathSeparator() to query the client OS instead of the server OS. I also see that we are using PlatformHelper.isUnderWindowsFamily() in EnvironmentOps too. That is also incorrect.

Please add helper methods to the EnvironmentDaemon to support this. And we must add an interface (OperatingSystemInspector) with these methods and that interface must be added to the RemoteObject.registerServer() call that exports the methods in EnvironmentDaemon. We will need to call RemoteObject.obtainInstance() in EnvironmentOps to get the new remote object proxy that can give us back our values.

For efficiency, please only call down to the client once per session. Save off the value (in the server side) the first time you call down and then just return that value over and over. Neither of these values will change during the user's session so we can keep them in the session context and reuse them.

Code Review 0325a

1. The isUnderWindowsFamily() does not need to be implemented on the client side. The getOperatingSystem() is enough, right? The rest of the computation can be done in the EnvironmentOps class.

2. The approach to call registerServer() 2 times in EnvironmentDaemon is incorrect. Instead, you would normally just pass an array of interfaces to a single call:

         modToken = RemoteObject.registerServer(new Class<?>[] { EnvironmentAccessor.class,
                                                                 OperatingSystemInspector.class },
                                                this,
                                                modToken,
                                                single);

However, we will split this off from EnvironmentDaemon (see below), so you don't have to make this change. I just want you to understand how you would do it if we had to.

3. It is not correct to use static vars for the data in EnvironmentOps.

   /** The name of the OS on the client side. */
   private static character clientOsName = null;

   /** The flag indicating Windows on client side. */
   private static logical isWindowsClient = null;

   /** The string representing delimiter between path entries on the client side. */
   private static String clientPathSeparator = null;

This is bad because these values must be context-local (they can be different for different clients). If you store them as static, the first client to connect will define this value for all clients which is not good.

4. You should not store these values as character or logical. Just store the OS name as a string and create a new character instance when EnvironmentOps.getOperatingSystem() returns its value. Likewise, the first time we call the client-side getOperatingSystem(), you can calculate the isWindowsOS value and store it as a context-local boolean.

5. We still need to honor the directory override of the OPSYS value in EnvironmentOps.getOperatingSystem(). Only if it is not there do we use the cached os.name (or call down to the client to get it).

6. Same thing as number 5 but for the directory override of the path separator in EnvironmentOps.getPathSeparator().

General Answers:

1. EnvironmentOps is the class that can be called from both server and client context, correct?

No. It is only server-side.

Currently EnvironmentDaemon has been initialized in ThinClient for Windows only(because it used only for INI/Registry handling). Do we need this class to be run in Linux client too? In this case may be better to separate Windows INI/Registry handling from other OS parameters inspection?

Yes, I think you are right. Put the client side in a different class.

Code Review 0326a

Everything here is much better. It is very close.

1. The changes in EnvironmentOps.fixupPropath() are not correct. I don't see why the previous code is wrong. Now that you have fixed the getPathSeparator() to properly honor the client's path sep (or one that the admin is explicitly put into the directory), everything should work fine. The code as you have changed it is definitely broken:

      // First select the right path separator
      String pathSep = null;

      // Windows case check first because Windows can have : in path string for drive letter
      if (propathInitial.indexOf(";") != -1)   <------ THIS CAN FAIL IF propathInitial is something like "c:\mypropathdir" 
      {
         pathSep = ";";
      }
      else if (propathInitial.indexOf(":") != -1)  <----- THIS CAN FAIL IF propathInitial is something like "/mypropathdir" 
      {
         pathSep = ":";
      }
      else
      {
         pathSep = Utils.getDirectoryNodeString(null, "path-separator", File.pathSeparator);  <---- this doesn't properly honor the client's path sep
      }

I think your changes just have to be removed. If you have some testcase that shows a problem, please discuss it here.

2. Please merge up to the latest bzr revision.

For note #1

The reason of this change is I've found if we just use as expected to be working:

...
String pathSep = getPathSeparator();
...

java.lang.ExceptionInInitializerError
    at com.goldencode.p2j.util.Registry.<init>(Registry.java:152)
    at com.goldencode.p2j.util.EnvironmentDaemon.<init>(EnvironmentDaemon.java:52)
    at com.goldencode.p2j.ui.chui.ThinClient.<init>(ThinClient.java:2436)
    at com.goldencode.p2j.ui.chui.ThinClient.initializePrep(ThinClient.java:2519)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:169)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:91)
    at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:219)
    at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:423)
    at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:111)
    at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:271)
Caused by: java.lang.NullPointerException
    at com.goldencode.p2j.util.EnvironmentOps.getPathSeparator(EnvironmentOps.java:717)
    at com.goldencode.p2j.util.EnvironmentOps.fixupPropath(EnvironmentOps.java:622)
    at com.goldencode.p2j.util.EnvironmentOps.<clinit>(EnvironmentOps.java:171)
    ... 10 more

In debug session I've found this is happening in ThinClient(boolean) constructor. Or in other thread running simultaneously with this constructor. Does it mean EnvironmentOps class is used on the client side?

2. Please merge up to the latest bzr revision.

OK.

The problem is this code in Registry.java:

      registryKeyName = "SOFTWARE"+NAME_SEPARATOR+"PSC"+NAME_SEPARATOR+"PROGRESS"+NAME_SEPARATOR+
                        EnvironmentOps.getVersion().toStringMessage();

Please eliminate this client-side usage of EnvironmentOps. Then the problem will go away.

The problem is this code in Registry.java:

 registryKeyName = "SOFTWARE"+NAME_SEPARATOR+"PSC"+NAME_SEPARATOR+"PROGRESS"+NAME_SEPARATOR+
 EnvironmentOps.getVersion().toStringMessage();

Please eliminate this client-side usage of EnvironmentOps. Then the problem will go away.

Yes, I also thought about this. There is another source - in OutputManager.java:

...
      if (EnvironmentOps.isBatchMode().booleanValue() ||
          (mgr != null && mgr.isBatchModeOverride()))
      {
...

This can be just removed I guess for client code:

...
      if (mgr != null && mgr.isBatchModeOverride())
      {
...

The other points after fix the client side is the problem with EnvironmentOps itself. On the class initialization stage we set up two propath related values. Both cases uses fixupPropath() method to replace path separator with ",". The fixupPropath() in turn calls the client down for path separator value. But on initialization stage the class is not ready to call the client because the link is not ready at this time. So we need to eliminate this dead lock before using path separator from the client side. Working on the fix.

For the point you noted the EnvironmentOps.getVersion().toStringMessage() can be replaced with string "10.2B". Is it OK?

No. We must be able to customize this value. That means calling "up" to the server to read a value from the directory.

No. We must be able to customize this value. That means calling "up" to the server to read a value from the directory.

OK.

I have localized server side issue in EnvironmentOps. If we move the propathOverride from the internal WorkArea class member to the static context - the problem disappears. And I think we can do this because the value of the propathOverride is only per the server instance. What do you think about this fix?

And I think we can do this because the value of the propathOverride is only per the server instance.

No, this is not correct. This value can be overridden for an account, group basis as well as at the server level. This is not safe for static use, it must be context-local.

I have localized server side issue in EnvironmentOps.

EnvironmentOps is designed only for server side usage. We should never try to use it on the client. Please use the remote directory APIs to access the directory from the client in this case.

I have found the regression with EnvironmentOps trying MAJIC conversion on my local system in Linux with last update:

...
./src/syman/dc/so-labr1.w
Exception: java.lang.NullPointerException
    ClearStream state:  clearCount = 0; inComment = no; inString = no; keepTildes = no
Scanning: &THEN
Scanning: LA(1)=' ' LA(2)='s'
java.lang.NullPointerException
    at com.goldencode.p2j.net.RemoteObject.obtainInstance(RemoteObject.java:1019)
    at com.goldencode.p2j.util.EnvironmentOps$ContextContainer.initialValue(EnvironmentOps.java:2098)
    at com.goldencode.p2j.security.ContextLocal$Fallback.initialValue(ContextLocal.java:477)
    at java.lang.ThreadLocal.setInitialValue(ThreadLocal.java:160)
    at java.lang.ThreadLocal.get(ThreadLocal.java:150)
    at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:163)
    at com.goldencode.p2j.util.EnvironmentOps$ContextContainer.obtain(EnvironmentOps.java:2085)
    at com.goldencode.p2j.util.EnvironmentOps.getSearchPathOverride(EnvironmentOps.java:553)
    at com.goldencode.p2j.preproc.Preprocessor.evaluate(Preprocessor.java:467)
    at com.goldencode.p2j.preproc.TextParser.condtext(TextParser.java:919)
    at com.goldencode.p2j.preproc.TextParser.aif(TextParser.java:655)
    at com.goldencode.p2j.preproc.TextParser.ppstatement(TextParser.java:306)
    at com.goldencode.p2j.preproc.TextParser.textBlock(TextParser.java:201)
    at com.goldencode.p2j.preproc.TextParser.text(TextParser.java:164)
    at com.goldencode.p2j.preproc.Preprocessor.<init>(Preprocessor.java:708)
    at com.goldencode.p2j.uast.AstGenerator.preprocess(AstGenerator.java:1242)
    at com.goldencode.p2j.uast.AstGenerator.prepareDataStream(AstGenerator.java:1022)
    at com.goldencode.p2j.uast.AstGenerator.prepareLexer(AstGenerator.java:1477)
    at com.goldencode.p2j.uast.AstGenerator.parse(AstGenerator.java:1394)
    at com.goldencode.p2j.uast.AstGenerator.processFile(AstGenerator.java:942)
    at com.goldencode.p2j.uast.AstGenerator.processFile(AstGenerator.java:814)
    at com.goldencode.p2j.uast.ScanDriver.scan(ScanDriver.java:203)
    at com.goldencode.p2j.uast.ScanDriver.scan(ScanDriver.java:122)
    at com.goldencode.p2j.convert.ConversionDriver.runScanDriver(ConversionDriver.java:386)
    at com.goldencode.p2j.convert.ConversionDriver.front(ConversionDriver.java:283)
    at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1728)
 scope --- #0:0./src/syman/dc/so-labr1.w{}{}
    OPSYS=(4,"null")
    SEQUENCE=(4,"null")
    OPEN-BROWSERS-IN-QUERY-DEFAULT-FRAME=(3,"{&OPEN-QUERY-br-clocklst}")
    LIST-1=(3,"fi-so-num fi-operation fi-routing fi-skill")
    DISPLAYED-OBJECTS=(3,"fi-date fi-time fi-emp-num fi-so-num fi-operation fi-routing fi-skill fi-emp-name fi-ecv-auth")
    DB-AWARE=(3,"no")
    ENABLED-FIELDS-IN-QUERY-BR-CLOCKLST=(3,"")
    LINE-NUMBER=(4,"null")
    INTERNAL-TABLES=(3,"tt-dcsfc")
    OPEN-QUERY-BR-CLOCKLST=(3,"OPEN QUERY br-clocklst FOR EACH tt-dcsfc NO-LOCK.")
    TABLES-IN-QUERY-BR-CLOCKLST=(3,"tt-dcsfc")
    FIELDS-IN-QUERY-BR-CLOCKLST=(3,"tt-dcsfc.co-num tt-dcsfc.timco-oper tt-dcsfc.oper-num tt-dcsfc.skill-code tt-dcsfc.skill-desc with no-label title " SO Oper Rtg Skill Description "")
    FRAME-NAME=(3,"DEFAULT-FRAME")
    BATCH-MODE=(4,"null")
    ENABLED-OBJECTS=(3,"fi-emp-num br-clocklst RECT-1")
    PROCEDURE-TYPE=(3,"Window")
    WINDOW-SYSTEM=(4,"null")
    SELF-NAME=(3,"br-clocklst")
    FILE-NAME=(4,"null")
    FIRST-TABLE-IN-QUERY-BR-CLOCKLST=(3,"tt-dcsfc")
    BROWSE-NAME=(3,"br-clocklst")
...

Looking for solution.

The update 0328a fixes the conversion issue I've found and works OK with and without another P2J server running simultaneously. The local conversion with and without update shows identical results for MAJIC converted code in Linux.

The different testing performed to check the validity of the 0328a update shows no difference in local conversion in Linux of the code with and without fixes. I have also performed native_library output and conversion check for Windows with this update and only difference in conversion is the "\" and "~\" are now converted to valid "\\" Windows path string. So I think if you do not have any notes the update is ready to go.

1. the EnvironmentOps.propathOverride is not used, it needs to be removed.
2. there is one more usage of EnvironmentOps on the P2J Client side. See the StanzaIni.getFullIniFileName:421, which calls EnvironmentOps.getLegacyFileSeparator().

Otherwise, I don't see problems with the logic.

Eugenie Lyzenko wrote:

The fixed update for review.

the EnvironmentOps.propathOverride is not used, it needs to be removed.

This can not be removed because is used internally in getSearchPath() method which in turn used outside EnvironmentOps class.

I'm refering to the EnvironmentOps.propathOverride field defined at line 151 (which has no javadoc, btw, and was added by your update), not the EnvironmentOps$WorkArea.propathOverride field (which is the one used in getSearchPath() and setSearchPath()).

Code Review 0403a

The only issue I see is that the class javadoc for ServerPropertiesDaemon is incorrect (due to copy/paste). Please fix that and then you can go into regression testing (both conversion and runtime are necessary).

With these changes, do the native library calls all work as expected on Windows?

With these changes, do the native library calls all work as expected on Windows?

The converted code and output files looks the same after this update. And because I've found previously it worked as expected the answer is yes, the native library calls all work as expected on Windows.

The conversion testing completed without regressions. The src/aero/timco/majic directory with converted files is identical after update applied. Starting the runtime regression tests.

Found the runtime regression. When the client starts, after enter username and login the following exception occurs:

...
java.lang.RuntimeException: Unresolvable remote export public abstract boolean com.goldencode.p2j.util.ServerPropertiesInspector.isBatchMode().
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.obtainRoutingKey(RemoteObject.java:1523)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1407)
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:97)
    at com.sun.proxy.$Proxy3.isBatchMode(Unknown Source)
    at com.goldencode.p2j.ui.client.OutputManager.initErrorWriter(OutputManager.java:333)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:219)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:93)
    at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:219)
    at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:423)
    at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:111)
    at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:271)
Caused by: java.lang.RuntimeException: No export or no access to com.goldencode.p2j.util.ServerPropertiesInspector:public abstract boolean com.goldencode.p2j.util.ServerPropertiesInspector.isBatchMode()
    at com.goldencode.p2j.net.HighLevelObject.getKey(HighLevelObject.java:121)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.obtainRoutingKey(RemoteObject.java:1495)
    ... 10 more
...

The problematic source is OutputManager.initErrorWriter():

...
      OutputManager mgr = instance();

      // This is the pointer to call up the server for properties
      ServerPropertiesInspector spInsp = null;

      if (session != null)
      {
         spInsp = (ServerPropertiesInspector) RemoteObject.obtainNetworkInstance(
                                                 ServerPropertiesInspector.class, session);
      }

      if ((spInsp != null && spInsp.isBatchMode()) ||
          (mgr != null && mgr.isBatchModeOverride()))
      {
         ErrorManager.initErrorWriter(new ErrorWriterBatch(ThinClient.getInstance()));
      }
...

Anyway because this is the pure client-side code I offer to remove asking server for a batch mode option in directory flag letting the client to choose run mode from client-side override:

...
   public static synchronized void initErrorWriter()
   {
      OutputManager mgr = instance();

      if (mgr != null && mgr.isBatchModeOverride())
      {
         ErrorManager.initErrorWriter(new ErrorWriterBatch(ThinClient.getInstance()));
      }
      else
      {
         ErrorManager.initErrorWriter(new ErrorWriterInteractive(ThinClient.getInstance()));
      }
   }
...

Is this change acceptable?

Eugenie Lyzenko wrote:

Found the runtime regression. When the client starts, after enter username and login the following exception occurs:

What you are missing is ACL configuration to access this resource from the client-side. This node needs to be added under the security/acl/net node:

          <node class="container" name="001550">
            <node class="strings" name="subjects">
              <node-attribute name="values" value="all_others"/>
            </node>
            <node class="netRights" name="rights">
              <node-attribute name="permissions" value="'0101'B"/>
            </node>
            <node class="resource" name="resource-instance">
              <node-attribute name="reference" value="com.goldencode.p2j.util.ServerPropertiesInspector"/>
              <node-attribute name="reftype" value="TRUE"/>
            </node>
          </node>

Eugenie, is it correct for the x86_64-w64-mingw32.zip to contain this path: x86_64-w64-mingw32/libs/, instead of x86_64-w64-mingw32/lib/?

Eugenie, is it correct for the x86_64-w64-mingw32.zip to contain this path: x86_64-w64-mingw32/libs/, instead of x86_64-w64-mingw32/lib/?

I do not understand a bit your question. What does it mean correct? The "libs" is artificial directory created manually. I put all binaries there and name means there are more than one library inside. When libffi building is done there is no such directory(and there is no "lib" directory too).

Eugenie Lyzenko wrote:

Eugenie, is it correct for the x86_64-w64-mingw32.zip to contain this path: x86_64-w64-mingw32/libs/, instead of x86_64-w64-mingw32/lib/?

I do not understand a bit your question. What does it mean correct? The "libs" is artificial directory created manually. I put all binaries there and name means there are more than one library inside. When libffi building is done there is no such directory(and there is no "lib" directory too).

OK, I was checking what are the steps to build P2J on windows. First I installed the mingw on 64 bits (following the instructions found here #1811-72 ). Second, I needed to setup libffi. For this, I expected that copying the entire contents of the x86_64-w64-mingw32.zip archive in my mingw64 folder will suffice, but I found that this wasn't enough: I had to manually move the contents of the x86_64-w64-mingw32/libs/ folder to the x86_64-w64-mingw32/lib/ folder. Thus: can you provide an archive which contains the minimal files, such that it can be safely extracted in the mingw64 or mingw32 folder? Else, please provide instructions about which libffi files need to be copied in which mingw folders (both 32 and 64 bit).

For this, I expected that copying the entire contents of the x86_64-w64-mingw32.zip archive in my mingw64 folder will suffice...

This is wrong assumption. The content of the zip files are the libffi build by mingw32. The root directories i686-pc-mingw32 and x86_64-w64-mingw32 inside the zip archives mean i686 and x86_64 builds made using mingw 32-bit version, not anything more. So to integrate libffi into current MinGW installation you need:

- Having MinGW for Windows 32 or 64 bit installed and functional.

- 64-bit(x86_64-w64-mingw32.zip archive)
1. Copy *.h files from archive include to target mingw_root/x86_64-w64-mingw32/include directory.
2. Copy all binaries from libs archive to target mingw_root/lib directory.

- 32-bit(i686-pc-mingw32.zip archive)
1. Copy *.h files from archive include to target mingw_root/i686-w64-mingw32/include directory.
2. Copy all binaries from libs archive to target mingw_root/lib directory.

That's it. Note the name for particular platform related build after mingw_root can vary for mingw, x86_64-w64-mingw32 and i686-w64-mingw32 examples from my local systems.

Greg, Constantin,

After a lot of debugging(including remote session to devbox01 P2J server and client) I need one point to clarify. The problematic code located on client side when OutputManager ask the server for batch mode flag set or not. To implement this I used the same tool as used to start main entry of the application:

1. Create class with static method and create appropriate interface describing this method.
2. Register network service by RemoteObject.registerStaticNetworkServer() call in StandardServer.bootstrap() method.
3. Obtain remote proxy by RemoteObject.obtainNetworkInstance() using the same session variable that used by client for main entry method.
4. Technically the calls for main entry and method call I need to execute is the same.

But the call isBatchMode() can not be mapped on the server side to appropriate static method. And the question is why and what I have missed here? Moreover when I run this on my local system(using simple/server and simple/client) - everything is OK. So additional question why it works differently on my local system and on devbox01? I'm using Oracle Java version 1.7.0_51-b13 JDK. The points 1-3 above are not enough for correct working?

Eugenie, have you added the ACL I mentioned at note 209? Because after adding this ACL, your update worked fine for me. The failure is at Dispatcher.getRoutingKey:

         NetResource ar = (NetResource) sessMgr.getSecurityPlugin("net");

         if (ar != null)
         {
            boolean allowed = canAdd ? ar.checkWriteAccess(group, method)
                                     : ar.checkReadAccess(group, method);

            if (!allowed)
            {
               return null;
            }
         }

Eugenie, have you added the ACL I mentioned at note 209?

For unknown reasons I have not got the e-mail with this your note in my e-mailbox. Will test this soon.

The main part of the runtime testing has passed without any regression. One time cycle, clean run(except known tc-job-002 issue). Testing the CTRL-C part.

Looks like I'm finishing with regression testing. Wait for finished CTRL-C results to arrive. And before committing P2J changes I need to clarify one thing.

I've made changes in (gso|rfq)-directory.xml.template file to be able to run client with updates.

1. I have to commit the majic changes first, into the staging branch, correct?
2. Do I need to modify all (gso|rfq)-directory.xml.* files or 2 files changes (gso|rfq)-directory.xml.template will be enough?

Eugenie Lyzenko wrote:

1. I have to commit the majic changes first, into the staging branch, correct?

Yes, you need to commit them in the staging branch.

2. Do I need to modify all (gso|rfq)-directory.xml.* files or 2 files changes (gso|rfq)-directory.xml.template will be enough?

Hm... not sure how is best to handle this, as this is a show-stopper change. They will certainly need to add this to the production directories when MAJIC will be brought up-to-date with the latest P2J. For now, I don't think is worth the trouble of updating the other directory files, updating the templates is enough.

Today I have got completely passed CTRL-C session tests(2-way and 3-way in single passed run). So I have started the uploading process. The directory templates are committed to the majic staging branch with log:

ae98410..a690eac  staging -> staging

Now I'll commit the P2J sources and send the update to the team.

The evl_upd20140403b.zip has been committed into bzr as 10509.