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

Fix bug in list_hp_retire function #10

Merged
merged 1 commit into from Apr 30, 2022
Merged

Conversation

kevinshieh0225
Copy link
Contributor

In hp_list project's function : list_hp_retire, it attempt to reorganize retire list while the object is deleting.
However, as I see, the origin code doesn't adjust the index simultaneously, which will skip track of the object
origin from &rl->list[iret + 1]. I'm not sure whether the behavior was originally intended or not.

@jserv
Copy link
Contributor

jserv commented Apr 23, 2022

@linD026, Please help review this proposed change.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Read https://cbea.ms/git-commit/ carefully. Then, write down informative commit message(s).

@kevinshieh0225 kevinshieh0225 changed the title Debug on function list retire Fix bug in list_hp_retire function Apr 23, 2022
@jserv
Copy link
Contributor

jserv commented Apr 23, 2022

You should use git rebase rather than git merge. Then, use git rebase -i to squash the commits and force-push.

In hp_list project's function : list_hp_retire, it attempts to reorganize
retire list while the object is deleting. However, as I see, the origin
code doesn't adjust the index simultaneously, which will skip track of
the object origin from &rl->list[iret + 1].

Change the behavior as:
 - If reorganize happens, keep the same index for next iteration.
 - Else, index++.
@linD026
Copy link
Collaborator

linD026 commented Apr 24, 2022

In my opinion, the original code might be a bug.
Since the delete is to move the objects after the iret forward one index, it will skip the next object when we increase the iret after every time deleted.

@kdnvt
Copy link

kdnvt commented Apr 29, 2022

I think the change is correct since the original code skips the next element if a deletion happens.

@jserv jserv merged commit 694204f into sysprog21:master Apr 30, 2022
@jserv
Copy link
Contributor

jserv commented Apr 30, 2022

Thank @kevinshieh0225 for contributing. The git commit message was amended upon the review of @linD026 and @kdnvt .

linD026 pushed a commit to linD026/concurrent-programs that referenced this pull request May 8, 2022
In list_hp_retire function of hp_list, the original code would skip the
next element if a deletion happens. It was incorrect because of its
neglect  to adjust the index simultaneously, skipping track of the
object origin from &rl->list[iret + 1].

This patch then changed the behavior as:
 - If reorganize happens, keep the same index for next iteration.
 - Else, index++.
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.

None yet

4 participants