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: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to