Skip to content

Added tests for the state machines #2

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 12, 2018
Merged

Added tests for the state machines #2

merged 3 commits into from
Feb 12, 2018

Conversation

@dettonijr
Copy link
Member

No description provided.

@dettonijr dettonijr mentioned this pull request Feb 1, 2018
Open
llewelld
llewelld reviewed Feb 6, 2018
View reviewed changes
tests/test_fsm.c Outdated

s = suite_create("Libpico");

// Base64 test case
Copy link
Member

Choose a reason for hiding this comment

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

Copy-paste error (not really Base64 test cases).

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

llewelld
llewelld reviewed Feb 6, 2018
View reviewed changes
tests/test_fsm.c Outdated

// We have to duplicate the objects because fsmpico tries to delete them later
cryptosupport_getprivateder(picoIdSKey, picoIdDer);
fsmpico_start(pico,
Copy link
Member

@llewelld llewelld Feb 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

All parameters should be on a single line to match the rest of the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

llewelld
llewelld approved these changes Feb 12, 2018
View reviewed changes
Copy link
Member

@llewelld llewelld left a comment

Choose a reason for hiding this comment

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

Did you manage to fix the temperamental test result? You mentioned it would occasionally fail randomly.

If so, it all looks good to me.

@dettonijr
Copy link
Member Author

I finally fixed the temperamental error.. It was a bit tricky.

When adding/removing something to the event queue using push_event I have a mutex to control the access (

libpico/tests/test_fsm.c

Line 87 in 746584a

pthread_mutex_lock(&queue_mutex);
), however, the current head of the queue might have been updated before getting to the mutex lock in that function. Then queue_head (copy of the actual pointer) would be invalid.

Now it is fixed. I will merge this

@dettonijr dettonijr merged commit acd1e6e into master Feb 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@llewelld llewelld llewelld approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dettonijr @llewelld