usnistgov / REFPROP-wrappers

Wrappers around NIST REFPROP for languages such as Python, MATLAB, etc.
193 stars 127 forks source link

Possible Incorrect Buffer Lengths in LabVIEW Wrapper #518

Closed DBTaylor closed 1 year ago

DBTaylor commented 1 year ago

It looks like some REFPROP calls in the LabVIEW wrapper are not properly initializing buffers or setting buffer lengths.

I do not have any incorrect behavior that I know is caused by this. The calls seem to be working correctly. I just want to verify correctness.

For example, in this call to REFPROP2dll: refprop2dll hFld, hIn and hOut are not padded to 10000, 255, and 255 respectively, but those are the lengths being passed. Also, the herr buffer is not being initialized, despite the DLL being given a length of 255.

If this is an issue, would it be preferable to set the lengths to match the input strings, or to pad the input strings?

dslauria commented 1 year ago

LabVIEW actually is padding to the numbers specified for those string lengths. If you open the Call Library Function node, you can see that the minimum length for each of those strings is defined by the length inputs.

image

It is possible to create a problem if the actual strings passed in are longer than those length values and the length values aren't updated manually. I can see where this could cause confusion, so I'll post an update later today. In the meantime, you can explicitly adjust the field lengths to your input string lengths like this:

image

If you do that, be sure to modify the minimum value field in the Call Library Function to a static default value that corresponds to those listed in the documentation. That will prevent issues if a blank string is passed in where you could end up with a length of zero, which would cause an error.

DBTaylor commented 1 year ago

Thanks for the clarification. I had no idea those minimum size configuration options existed in the node

dslauria commented 1 year ago

A new updated LabVIEW driver will be posted shortly. This version includes very explicit string lengths, padding with spaces, and optional null termination. It turns out that the dll ignores length value inputs like herr_length so those values have been removed from the output clusters to avoid confusion. Hopefully this new wrapper will avoid confusion and add some stability. These updates apply only to the high level API. The lower level VIs are unsupported but could be updated using the same technique.