Skip to content

Conversation

@bgilbert
Copy link
Collaborator

@bgilbert bgilbert commented Apr 17, 2025

FrameOfReferenceToDisplayedCoordinateSystemTransformationMatrix is 63 characters and we need room for the trailing NUL. While the compiler would have added a trailing 0 byte anyway to maintain struct alignment, reading the string would invoke undefined behavior.

Fixes a warning on GCC 15:

../src/dicom-dict-tables.c:3296:33: warning: initializer-string for array of ‘char’ truncates NUL terminator but destination lacks ‘nonstring’ attribute (64 chars into 63 available) [-Wunterminated-string-initialization]
 3296 |     {0X0070030B, DCM_VR_TAG_FD, "FrameOfReferenceToDisplayedCoordinateSystemTransformationMatrix"}

@bgilbert bgilbert marked this pull request as draft April 17, 2025 20:51
@bgilbert bgilbert marked this pull request as ready for review April 17, 2025 23:31
"FrameOfReferenceToDisplayedCoordinateSystemTransformationMatrix" is 63
characters and we need room for the trailing NUL.  While the compiler
would have added a trailing 0 byte anyway to maintain struct alignment,
reading the string would invoke undefined behavior.

Fixes a warning on GCC 15:

    ../src/dicom-dict-tables.c:3296:33: warning: initializer-string for array of ‘char’ truncates NUL terminator but destination lacks ‘nonstring’ attribute (64 chars into 63 available) [-Wunterminated-string-initialization]
     3296 |     {0X0070030B, DCM_VR_TAG_FD, "FrameOfReferenceToDisplayedCoordinateSystemTransformationMatrix"}
@bgilbert
Copy link
Collaborator Author

Actually this isn't so harmless, since the one-byte overread is undefined behavior. Updated the changelog and description.

@jcupitt jcupitt merged commit a425123 into ImagingDataCommons:main Apr 18, 2025
6 checks passed
@jcupitt
Copy link
Collaborator

jcupitt commented Apr 18, 2025

Ooop! Thank for this Benjamin.

@bgilbert bgilbert deleted the nul branch April 18, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants