Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Feb 11, 2015

The assert() macro is needed for utlist (which is used in #2285 and #2404). Therefore we need an implementation of assert().

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Feb 11, 2015
/**
* @see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/assert.h.html
*
* @todo implement behaviour for NDEBUG defined.
Copy link
Member

Choose a reason for hiding this comment

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

Reading the docs, you already implemented behaviour for NDEBUG defined?

@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Addressed comments

@thomaseichinger
Copy link
Member

ACK when squashed.

/**
* @see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/assert.h.html
*
* @todo implement behaviour for NDEBUG not defined.
Copy link
Member

Choose a reason for hiding this comment

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

Why not implement the rest right now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I was not sure if abort() is implemented on msp430

@miri64 miri64 force-pushed the msp430/api/add-assert branch from 3bb8ab1 to 722a9e7 Compare February 12, 2015 11:49
@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Added implementation. Decided to use DEBUGF instead of printf, to save the stack memory (also so I do not need to implement the formatting ^^), in case it is called. If one wants to enable output they can always use ENABLE_DEBUG.

* @def assert
* @see http://pubs.opengroup.org/onlinepubs/9699919799/functions/assert.html
*
* @todo implement behaviour for NDEBUG not defined.
Copy link
Member

Choose a reason for hiding this comment

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

done?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Removed this line

@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Addressed comments

@haukepetersen
Copy link
Contributor

ACK when squashed.

@miri64 miri64 force-pushed the msp430/api/add-assert branch from d0ed730 to f00e532 Compare February 12, 2015 13:08
@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Squashed. Waiting for travis

@miri64 miri64 force-pushed the msp430/api/add-assert branch from f00e532 to 9077299 Compare February 12, 2015 13:24
@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Addressed @LudwigOrtmann's post-ACK comment. Can we merge it now…

@LudwigKnuepfer
Copy link
Member

ACK ACK

@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Waiting for travis

miri64 added a commit that referenced this pull request Feb 12, 2015
@miri64 miri64 merged commit f3fccf2 into RIOT-OS:master Feb 12, 2015
@miri64 miri64 deleted the msp430/api/add-assert branch February 12, 2015 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants