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

SineExtraction::init() doesn't reset mBuf #236

Closed
AlexHarker opened this issue Aug 11, 2023 · 8 comments
Closed

SineExtraction::init() doesn't reset mBuf #236

AlexHarker opened this issue Aug 11, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@AlexHarker
Copy link
Contributor

AlexHarker commented Aug 11, 2023

The contents of mBuf should have a consistent size, but if it isn't reset when init() is called this can not be the case (leading to possible crashes or undefined behaviour).

When this object was updated the reset was commented out, but it is needed, so I would propose that:

// mBuf = std::queue<ArrayXcd>();

Should be:

mBuf = makeEmptyQueue(alloc);

Doing so solves an eigen_assert I get with debug builds. @weefuzzy ?

@AlexHarker AlexHarker added the bug Something isn't working label Aug 11, 2023
@weefuzzy
Copy link
Member

Yes. Or just call clear()? Horses for courses.

@AlexHarker
Copy link
Contributor Author

I don't see a clear() in SinExtraction - am I missing something?

@AlexHarker
Copy link
Contributor Author

oh sorry - on mBuf - I don't mind either way but it asserts without something and is liable to crash, so it would be good to fix. @weefuzzy happy to PR whichever you'd prefer.

@weefuzzy
Copy link
Member

Isn't mBuf a deque with a clear()? In any case, like I said, horses for courses.

@weefuzzy
Copy link
Member

Cool, thanks. Very slight aesthetic preference to calling clear().

@AlexHarker
Copy link
Contributor Author

Turns out that clear() is not a thing for a queue.

@weefuzzy
Copy link
Member

Well then, that makes our choice.

@AlexHarker
Copy link
Contributor Author

Fixed in 7247a36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants