Skip to content

Improved error checking of snprintf - #19

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:masterfrom
seanm:snprintf-error-checks
Jan 12, 2026
Merged

Improved error checking of snprintf#19
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:masterfrom
seanm:snprintf-error-checks

Conversation

@seanm

@seanm seanm commented Jun 18, 2025

Copy link
Copy Markdown
Collaborator

Although usually snprintf return values are ignored, there were a few places that bothered to check, and a recent change stored the return value in an unsigned, making it hard to check to a negative result (error condition).

This reworks the function a little bit to improve error checking.

@seanm

seanm commented Jun 18, 2025

Copy link
Copy Markdown
Collaborator Author

@issakomi care to review?

I haven't actually tested this yet, but wanted to share...

@seanm

seanm commented Jun 19, 2025

Copy link
Copy Markdown
Collaborator Author

OK, I tested this by trying a few commands to nifti1_tool and seems to work as before.

@seanm

seanm commented Jul 7, 2025

Copy link
Copy Markdown
Collaborator Author

@issakomi friendly ping...

@issakomi

issakomi commented Jul 8, 2025

Copy link
Copy Markdown
Member

Thank you. I noticed this while looking through the Nifti update. I didn't test.

Although usually snprintf return values are ignored, there were a few places that bothered to check, and a recent change stored the return value in an unsigned, making it hard to check to a negative result (error condition).

This reworks the function a little bit to improve error checking.
@seanm
seanm force-pushed the snprintf-error-checks branch from 4574d05 to bacab0f Compare January 2, 2026 01:33
@seanm

seanm commented Jan 4, 2026

Copy link
Copy Markdown
Collaborator Author

@hjmjohnson @issakomi I think we can merge this now?

@hjmjohnson
hjmjohnson merged commit 5b2657f into InsightSoftwareConsortium:master Jan 12, 2026
5 checks passed
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.

3 participants