Bogus cast introduced in PR 6690

[bzbarsky-apple]

Problem

#6690 has a commit entitled "Fix type conversion error" which "fixes" the error by adding a reinterpret_cast from uint8_t** to ByteSpan*.

If we assume write is true, we are now going to read the the data pointer and size from the Span so we can do the memcpy. The data pointer we will read is the uint8_t* value. The size we will read is ... looks like some combination of the FabricConnected, OffPremiseServicesReachableIPv4, OffPremiseServicesReachableIPv6, and the random padding byte. This is unlikely to end well.

If write is false, we will read whatever ByteSpan value was passed in and try to write it to the struct. We'll write its data pointer to the uint8_t*, write the size to the booleans and padding byte, then later try to treat the uint8_t* as a Pascal string and bad things will happen.

Proposed Solution

Depending on how close we are to supporting CHAR_STRING in structs (@vivien-apple ?), one of:

1. Enable that support and remove the cast.
2. Revert Add initial version of the General Diagnostics Cluster #6690
3. Switch this member to OCTET_STRING for now until CHAR_STRING is enabled, add comment and file followup issue.

And stop adding reinterpret_casts just to get things to compile....

@yufengwangca @andy31415 @woody-apple

[yufengwangca]

This issue was introduced from #6596.

• chip::ByteSpan * {{name}}Span = &entry->{{name}}; // {{type}}

Is this addition valid?

[bzbarsky-apple]

And if the new cluster had been added to src/darwin/Framework/CHIP/chip-tool.zap, which actually tests this sort of thing in CI, this would have been caught, fwiw.... Also would have caught it not being added to src/controller/controller-clusters.zap.

Also would have been good to add to examples/chip-tool/chip-tool.zap

[bzbarsky-apple]

Is this addition valid?

It's correct for OCTET_STRING. CHAR_STRING, again, is not supported in structs yet.

[yufengwangca]

My new cluster does not get added to chip-tool, this issue was discovered from any server side zaps.

[yufengwangca]

I prefer taking out item "Name" from structure for now and add comments & file a followup issue.

[bzbarsky-apple]

Filed #6805 to change the type back to CHAR_STRING when we can.