pnoltes commented on PR #699: URL: https://github.com/apache/celix/pull/699#issuecomment-1894351370
> When I modified `TEST_F(JsonSerializerTests, ParseTests)` as following, a crash happened: > > ```c++ > type = nullptr; > inst = nullptr; > rc = dynType_parseWithStr(example5_descriptor, nullptr, nullptr, &type); > ASSERT_EQ(0, rc); > rc = jsonSerializer_deserialize(type, example5_input, strlen(example5_input), &inst); > ASSERT_EQ(0, rc); > > json_auto_t* result = nullptr; > // crash happens HERE! > rc = jsonSerializer_serializeJson(type, inst, &result); > ASSERT_EQ(0, rc); > ``` > > I found two issues: > > 1. In `example5_input`, the `node` pointed to by `head` misses `value` member. Similarly, the `node` pointed to by `head->left` misses `right` member. > 2. `nullptr` is not dealt in `jsonSerializer_serializeJson`, which leads to the crash. > > ```c++ > case '*' : > subType = dynType_typedPointer_getTypedType(type); > status = jsonSerializer_writeAny(subType, *(void **)input, &val); // dereference null ptr > break; > ``` > > For the first issue, IMO all struct members should be present in the json object, because C struct is very rigid, unlike json object. Our data model is based on C struct not on flexible json object. > > For the second issue, I think `nullptr` should also be serialized/deserialized. > > WDYT? @pnoltes I agree with both. And making a nullptr possible is good improvement I had not considered. Concerning JSON, If you treat the descriptor as a low level (JSON) schema, it is also logical that the expected JSON should have the expected members. -- 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
