pnoltes commented on code in PR #782:
URL: https://github.com/apache/celix/pull/782#discussion_r1929836087
##########
libs/dfi/README.md:
##########
@@ -71,9 +71,9 @@ The data types supported by the interface description include:
*Type schema*:
- |**Identifier**|B |D |F |I |J |S |V |Z
|b | i | j | s |P | t |N |
-
|---------|---|------|-----|-------|-------|-------|----|--------------|-----|--------|--------|--------|------|------------------|---|
-
|**Types**|char|double|float|int32_t|int64_t|int16_t|void|boolean(uint8)|uchar|uint32_t|uint64_t|uint16_t|void
*| char *(C string) |int|
+ |**Identifier**|B |D |F |I |J |S |V |Z
|b | i | j | s |P | t |N | p
| a |
Review Comment:
Using JSON Schema is indeed a good option. It helps avoid overly defensive
programming and ensures validation occurs before messages are deserialized.
However, isn’t the DFI descriptor already a form of serialization schema?
But less focused on the serialized format itself and more on the memory
representation (excluding padding and alignment details, which are handled by
libffi).
Given that libjansson does not support JSON Schema, I believe it would be
better to extend the DFI type system to be more explicit. This could also open
the door to future enhancements, such as adding support for annotations like
`min`/`max`.
One potential downside of this approach is that it would no longer be
possible to generate a descriptor string directly from a header file.
Currently, - I think - this is possible (e.g., parsing a `celix_array_list_t*`
field by adding an `a` descriptor type). This is why I think it might be better
to replace `celix_array_list_t` with type-specific versions.
For now, my proposal is to keep the `a` descriptor for this pull request. If
we decide to replace `celix_array_list_t` with type-specific versions in the
future, both the DFI and serialization logic would need to be updated
accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
