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

Rollback partial wrong implemented code in pr #89 #114

Merged
merged 3 commits into from
Feb 19, 2023

Conversation

EagleTw
Copy link
Collaborator

@EagleTw EagleTw commented Feb 19, 2023

No description provided.

src/emulate.c Fixed Show fixed Hide fixed
@EagleTw
Copy link
Collaborator Author

EagleTw commented Feb 19, 2023

Suggestion in CWE-468
--> Always use array indexing instead of direct pointer manipulation.

I do not think there is a way to us array indexing here.

@gagachang
Copy link
Contributor

gagachang commented Feb 19, 2023

How about using union:

union {
    uint32_t csr_time_32[2];
    uint64_t csr_time_64;
};

@EagleTw
Copy link
Collaborator Author

EagleTw commented Feb 19, 2023

Hi @gagachang,

Maybe this new change has a better readability.
What do you think?

@gagachang
Copy link
Contributor

Yes, it looks better. Minor suggestion:

rv->csr_time[0] = t & 0xFFFFFFFF;

Actually, I think two dedicated variables for time is OK, since rv32emu targets RV32 system.
The spec defines two CSRs for time on RV32 system.

uint32_t csr_time;
uint32_t csr_timeh;

@EagleTw
Copy link
Collaborator Author

EagleTw commented Feb 19, 2023

Hi @gagachang
Thank you for your code review and suggestion

@jserv jserv merged commit fd4de56 into sysprog21:master Feb 19, 2023
2011eric pushed a commit to 2011eric/rv32emu that referenced this pull request Jul 22, 2023
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.

3 participants