Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be careful with snprintf #222

Merged

Conversation

rwalker-apple
Copy link
Contributor

Problem

ErrStr puts too much stock in snprintf's behavior and return value,
and may as a result may index outside the bounds of the output buffer

consider:
http://www.cplusplus.com/reference/cstdio/snprintf/
http://www.cplusplus.com/reference/cstdio/snprintf/

Summary of Changes

restructure to be able to safely ignore sprintf's return value

@gerickson
Copy link
Contributor

I have it on my to-do list today to land unit tests for this facility. Would you be OK pending this until I land that PR?

@rwalker-apple
Copy link
Contributor Author

I have it on my to-do list today to land unit tests for this facility. Would you be OK pending this until I land that PR?

does the unit test framework support mocking or handle exceptions (crashes)?

@gerickson
Copy link
Contributor

I have it on my to-do list today to land unit tests for this facility. Would you be OK pending this until I land that PR?

does the unit test framework support mocking or handle exceptions (crashes)?

Probably a longer conversation, but maybe. When you do 'make check' the build system framework will treat a crash as a fail and log it as such.

If you are using the super-simple-it-runs-on-bare-metal-devices nl-unit-test framework, it doesn't catch exceptions or handle backtraces or anything of the sort. A crash will crash the program. You could make arguments one way or another whether this is good or bad. But, in our world, whether it's a Linux-class device or a bare-metal-class RTOS device, we have a unit test image that gets loaded and the unit tests run, with the same core framework and the same unit tests, on device.

In short, there was more value to having one thing that ran everywhere (on target and on build host as well as Linux- and bare-metal-class) than having something fancier and runtime and platform-specific.

Copy link

@hawk248 hawk248 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@woody-apple woody-apple merged commit 96e47fe into project-chip:master Apr 2, 2020
@rwalker-apple rwalker-apple deleted the Be-careful-with-snprintf branch April 2, 2020 22:02
@rwalker-apple
Copy link
Contributor Author

@gerickson this got merged before any tests were imported. Are there tests that still need to come in or should I be adding those to what you've landed for this directory?

@rwalker-apple rwalker-apple mentioned this pull request Apr 4, 2020
@gerickson
Copy link
Contributor

@gerickson this got merged before any tests were imported. Are there tests that still need to come in or should I be adding those to what you've landed for this directory?

I added these from openweave-core but they are arguably not proper unit tests and could stand improvement.

mkardous-silabs referenced this pull request in mkardous-silabs/connectedhomeip Nov 2, 2022
…r neighborhood discovery protocol (project-chip#23105)" from silabs to silabs_1.0

Merge in WMN_TOOLS/matter from cherry-pick/add-lwip-ipv6-mld-support-new to silabs_1.0

Squashed commit of the following:

commit 61fdbeb2b50a67a05ed27de87ed052453d9bfe5e
Author: Rohan Sahay <103027015+rosahay-silabs@users.noreply.github.com>
Date:   Tue Oct 11 19:51:03 2022 +0530

    [EFR32] Adds lwip ipv6 MLD support for neighborhood discovery protocol (project-chip#23105)

    * Adds LWIP_IPV6_MLD support for Neighborhood discovery

    * Adds temporary fix for NS loopback issue
rerasool added a commit to SiliconLabs/matter that referenced this pull request Nov 2, 2022
…D support for neighborhood discovery protocol (project-chip#23105)" from silabs to silabs_1.0

Merge in WMN_TOOLS/matter from cherry-pick/add-lwip-ipv6-mld-support-new to silabs_1.0

Squashed commit of the following:

commit 61fdbeb2b50a67a05ed27de87ed052453d9bfe5e
Author: Rohan Sahay <103027015+rosahay-silabs@users.noreply.github.com>
Date:   Tue Oct 11 19:51:03 2022 +0530

    [EFR32] Adds lwip ipv6 MLD support for neighborhood discovery protocol (project-chip#23105)

    * Adds LWIP_IPV6_MLD support for Neighborhood discovery

    * Adds temporary fix for NS loopback issue
Sarthak-Shaha pushed a commit to Sarthak-Shaha/connectedhomeip_silabs that referenced this pull request Jan 28, 2025
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.

5 participants