Skip to content

Conversation

Astabol
Copy link
Contributor

@Astabol Astabol commented Aug 5, 2025

Hello @qwsdl , This is the fixed for Fixed Issue #205.
Capture

Thank You

@deRemo
Copy link
Member

deRemo commented Aug 7, 2025

Thanks @Astabol, please remove the comments you have introduced, since your changes are self-explanatory

@Astabol
Copy link
Contributor Author

Astabol commented Aug 7, 2025

Sure. I shall remove. Thanks

@Astabol
Copy link
Contributor Author

Astabol commented Aug 8, 2025

Hello @deRemo, I removed the comments. Thank You

@deRemo
Copy link
Member

deRemo commented Aug 8, 2025

Thanks @Astabol, but please squash the commits into one for a cleaner git history

I generally use git rebase -i HEAD~X where X represents the number of commits you want to go back in your history to re-write. Then fix-up the latest commits to the previous one

More info: https://garrytrinder.github.io/2020/03/squashing-commits-using-interactive-rebase

@Astabol
Copy link
Contributor Author

Astabol commented Aug 8, 2025

Hi @deRemo , I squashed the commits to single. Thank You

Copy link
Member

Choose a reason for hiding this comment

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

Is this required for issue #205? I'd rather not touch the base simulation engine: I'm pretty sure that we want the simulator to fail if getIncomingEvents() is null-pointer, rather than "hiding" it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I understand.

Copy link
Member

Choose a reason for hiding this comment

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

As my comment below, I'm not sure that hiding the nullpointer is the right approach. When calling getCurrentRequestedMips we basically assume that the Vm has been placed, hence getHost() shouldn't be null. I believe that the issue to be fixed is more intricate than this...

Copy link
Contributor Author

@Astabol Astabol Aug 10, 2025

Choose a reason for hiding this comment

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

Yeah, it is not fixing the root cause. If anyone called this from another example might get the same issue. I will look into it. Thank You

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your time!

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.

2 participants