Remove variable arguments from C-API

[modelica-trac-importer]

Reported by tsommer on 6 Jun 2018 08:09 UTC
Currently it is not possible to call or implement functions with a variable number of arguments in languages
and libraries that bind to functions in shared libraries dynamically like Java Native Access,
Python ctypes and .NET P/Invoke.

In order to allow these platforms to fully support the FMI API, I suggest to remove all variable arguments lists from the C-API.

Currently variable arguments are only used in one callback function (fmi2FunctionTypes.h, line 124):

typedef void (*fmi2CallbackLogger)(fmi2ComponentEnvironment, fmi2String, fmi2Status, fmi2String, fmi2String, ...);

The same result can be achieved by formatting the message string using sprintf() in the FMU and passing it to the callback as a single argument.

---

Migrated-From: https://trac.fmi-standard.org/ticket/435

[modelica-trac-importer]

Comment by andreas.junghanns on 6 Jun 2018 09:37 UTC
This request is asking to make the FMU code more difficult to write (e.g. memory handling) that now to make the importers life easier (no need to handle variable number of arguments).

Who should we make life easier: Exporters or importers?

[modelica-trac-importer]

Comment by tsommer on 6 Jun 2018 13:17 UTC
This is not about making life easier for the (many) importers to make it harder for the (fewer) exporters but to allow (some) importers to use arguments that are currently "unreachable".

[modelica-trac-importer]

Comment by Andreas Nicolai on 6 Jun 2018 16:45 UTC
Replying to [comment:1 Andreas Junghanns (QTronic)]:

This request is asking to make the FMU code more difficult to write (e.g. memory handling) that now to make the importers life easier (no need to handle variable number of arguments).

Who should we make life easier: Exporters or importers?

I can only imagine a scenario for the exporting FMU where some of the
strings to be passed back are fixed character constants and one wants to
avoid to allocate a chunk of memory in the exporting FMU to fit the
entire concatenated string. This could only pose a problem for very
limited hardware (i.e. embedded systems). But surely, one would not
expect very verbose (prosa form) error messages from such a system,
right? But, instead error codes or abbreviated msgs would be used so
that such a limited FMU would only need to allocate a fixed "error
message storage location" once where all strings could be assembled
therein.

For the importing side things could be resolved equally simple by
writing a small native C callback function that handles the
concatenation and passes this on to the language-specific callback... a
little more effort, surely, but also manageable.

Still, in my oponion, the request is valid - don't make an API more
complicated than necessary, and IMHO variable argument lists are not a
necessity in this context.

-Andreas

[modelica-trac-importer]

Comment by pierre.mai on 12 Jun 2018 21:11 UTC
FWIW I completely agree with this proposed change, since the current definition really is a bad idea:

• Variadic functions cannot portably be wrapped in C

  FMU code that wants to make use of the callback always has to call it directly, and cannot create its own variadic logging function, since it cannot pass VAs onto the callback (since logger is like printf, we are missing the equivalent vprintf function for passing on varargs).

  So arguably, if there were not other problems with variadic functions in DLL/SO interfaces, logger should be specified like vprintf and not printf.

• The current function is underspecified

  It states for the logger callback:

  "Argument “message” is provided in the same way and with the same format control as in function “printf” from the C standard library."

  However since no specific C standard is referenced in the FMI spec, this leaves the exact semantics, and especially the format controls and flags that have to be supported open to interpretation.

• Requires complete printf implementation in calling environment

  When calling FMUs from environments that are not implemented in C, not only does the calling environment have to support variadic functions in its foreign function interface (as tsommer already pointed out), but it must also support a printf implementation that can decode the control string and map the varargs accordingly.

So not only is the current definition inconvenient for a number of importing tools, it is also inconvenient for FMU writers (especially those FMUs which are manually programmed and not auto-generated, since of course auto-generators can inline calls to the callback at all appropriate places).

Additionally there have been quite a few instances where ABI implementations between compilers have had subtle differences in handling of stack alignment for variadic functions, so the current approach also risks subtle stack or memory corrupting bugs in those cases (this in itself is not a compelling argument, but adds to my weariness in having variadic functions in cross-compiler calls ... I've spent too much blood on dealing with these issues on various PowerPC ABIs for example).

---

Handling of variable references

On a related note, the facility for referencing variables through their value references also seems problematic, since VRs do not actually uniquely identify variables (=> aliases), so the simulation environment cannot know which variable name to use in those cases (this was more clear in FMI 1.0, where one of the aliases was to be called out as not an alias, but for FMI 2.0 this restriction no longer exists). It is also unclear what advantages this offers, since the FMU itself should be able to know its variable names (and changing them in the modelDescription.xml after generation is not allowed). So unless I'm missing a compelling reason for this added complexity (which I very well might, BTW), I think this needs at least re-design or clarification.

[modelica-trac-importer]

Comment by hansolsson on 6 Jul 2018 12:09 UTC
As far as I understand filtering of messages is mostly done before calling the logger-function - that's why we have a separate fmi2SetDebugLogging-function.

IF that weren't the case the variable number of argument variants would have an advantage in terms of allowing an implementation to delay formatting messages until it knows they will actually be logged.

As I understand it variable number of argument functions are problematic for safety reasons (and there exists a number of variants to reduce this). The simplest mistake would be when we have a pre-formatted message and use logger(..., message) instead of logger(..., "%s", message).

Thus I see that it would make sense to use a fixed number of arguments function in the interface.

[modelica-trac-importer]

Comment by Andreas Nicolai on 6 Jul 2018 12:30 UTC
Is there actually a rationale for providing the arguments separately to the logging function?

In my opinion there are only two valid use cases that warrant the splitting of text and arguments:

1. internationalization/translation purposes
2. different accuracy for floating point numbers depending on display context

The first is clearly out of focus, since it would require much more conventions for message strings generated by FMUs to work.

The second is more or less artificial, since the printf() format specifications already allow definition of precision/number formats and those would then be overruled by, for example, the GUI that displays the message. But that overruling of number formatting may not be in the interest of the FMU that generated the message in the first place (just think about layout problems, where the FMU tries to generate a concise table-like output where fixed number sizes are used).

Hence, both use cases are not applicable. Personally, I see no use case to split message and arguments in the first place. Do you?

If there is no valid reason, please change the logger interface into a simple function that provides a complete message string and a fixed number of arguments!

-Andreas

[modelica-trac-importer]

Comment by dag.bruck on 6 Jul 2018 12:50 UTC
Variadic functions should be avoided if at all possible. They are a common source of errors, emphasized by the fact that Koenig's C Traps and Pitfalls devotes an entire appendix to explain them.

(As a side note, MSL got it wrong in at least two places.)

[modelica-trac-importer]

Comment by anonymous on 25 Jul 2018 14:05 UTC
EDF is totally in favor to remove variadic functions. They cannot be used properly from Java using JNA and the log callback is working wrongly in our open source tool JavaFMI (https://bitbucket.org/siani/javafmi) as variadic arguments cannot be retrieved.

[modelica-trac-importer]

Comment by beutlich on 13 Aug 2018 07:41 UTC
Replying to [comment:7 dag.bruck@…]:

(As a side note, MSL got it wrong in at least two places.)

Do you mean the availability or the usage of the Modelica utility functions ModelicaFormatMessage / ModelicaFormatError / ModelicaFormatWarning? In case of the availability it is a Modelica specification issue and not a library issue (though MSL still could avoid to use these functions).

[pmai]

I think this should be decided soon by polling:

A) Leave as is

typedef void (*fmi2CallbackLogger)(fmi2ComponentEnvironment componentEnvironment,
                      fmi2String instanceName,
                      fmi2Status status,
                      fmi2String category,
                      fmi2String message, ...);

B) Remove VarArgs, but leave in #type-style variable references

typedef void (*fmi2CallbackLogger)(fmi2ComponentEnvironment componentEnvironment,
                      fmi2String instanceName,
                      fmi2Status status,
                      fmi2String category,
                      fmi2String message);

C) Remove VarArgs and #type-style variable references, i.e. only a simple uninterpreted string is passed

typedef void (*fmi2CallbackLogger)(fmi2ComponentEnvironment componentEnvironment,
                      fmi2String instanceName,
                      fmi2Status status,
                      fmi2String category,
                      fmi2String message);

[andreas-junghanns]

The difference between B) and C) is that the message cannot contain place holders with the value references any more, right? This would require that all FMUs know the name of the variable to create human readable messages. One of the early design rationales was that use an external modelDescription.xml to "lighten" the FMU and it must not know details about the variables, such as names. Are we giving up on that? Why? Or did I miss something in the arguments?

[pmai]

Yes, that would be the difference between B) and C). The problem with the current approach is that a VR does not uniquely identify a variable name to print due to aliasing (see my comments above from 12. June):

On a related note, the facility for referencing variables through their value references also seems problematic, since VRs do not actually uniquely identify variables (=> aliases), so the simulation environment cannot know which variable name to use in those cases (this was more clear in FMI 1.0, where one of the aliases was to be called out as not an alias, but for FMI 2.0 this restriction no longer exists). It is also unclear what advantages this offers, since the FMU itself should be able to know its variable names (and changing them in the modelDescription.xml after generation is not allowed). So unless I'm missing a compelling reason for this added complexity (which I very well might, BTW), I think this needs at least re-design or clarification.

Since the vrs will now be unique across types, the vr place holder mechanism will require a redesign in any case, so if that design rationale is still taken to be worth the effort, we should desing a replacement, or we should do away with the current one.

Personally I'd rather have the knowledge which variable names to use in the FMU than the environment, but if it is deemed worthwhile to split that knowledge from the FMU, I'd give this rationale in the document.

[beutlich]

No more poll needed.