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.

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

Remove deprecated BPatch_addressSpace::findFunctionByAddr #820

Closed
hainest opened this issue Aug 29, 2020 · 4 comments · Fixed by #837
Closed

Remove deprecated BPatch_addressSpace::findFunctionByAddr #820

hainest opened this issue Aug 29, 2020 · 4 comments · Fixed by #837
Assignees
@hainest
Labels
API-BREAKER This change breaks the public API BPatch This issue is directly related to BPatch code cleanup Bring the code up to modern standards or remove deprecated features
Milestone
Release 11.0

Comments

@hainest
Copy link
Contributor

hainest commented Aug 29, 2020

This was deprecated by 7f20129 in 2010. However, it is still being used by BPatch_addressSpace::findFunctionByAddr. I'm not sure if it can actually be removed.

@sashanicolas @mxz297 Is this safe to remove, or should we undeprecate it?
@hainest hainest added code cleanup Bring the code up to modern standards or remove deprecated features BPatch This issue is directly related to BPatch API-BREAKER This change breaks the public API labels Aug 29, 2020
@hainest hainest added this to the Release 10.3 milestone Aug 29, 2020
@hainest hainest self-assigned this Aug 29, 2020
@mxz297
Copy link
Member

mxz297 commented Aug 30, 2020

I feel findFunctionByAddr seems like a useful operation to keep.

@hainest
Copy link
Contributor Author

hainest commented Aug 30, 2020

@mxz297 Sounds reasonable to me. I'll undeprecate it.

@hainest hainest changed the title BPatch_addressSpace::findFunctionByAddr is deprecated? Undeprecate BPatch_addressSpace::findFunctionByAddr Aug 30, 2020
@hainest hainest removed the API-BREAKER This change breaks the public API label Aug 30, 2020
@hainest
Copy link
Contributor Author

hainest commented Aug 30, 2020

Ah, no. This should be deprecated. There is a warning message (https://github.com/dyninst/dyninst/blob/master/dyninstAPI/src/BPatch_addressSpace.C#L713) which indicates that function will return an arbitrary result if there is more than one function found, and that there are suitable replacements to use, instead.

@hainest hainest changed the title Undeprecate BPatch_addressSpace::findFunctionByAddr Remove deprecated BPatch_addressSpace::findFunctionByAddr Sep 6, 2020
@hainest hainest mentioned this issue Sep 7, 2020
Merged
@hainest hainest added the API-BREAKER This change breaks the public API label Sep 7, 2020
@hainest hainest modified the milestones: Release 10.3, Release 11.0 Sep 7, 2020
@hainest
Copy link
Contributor Author

hainest commented Sep 8, 2020

Merged into api_breakages via #837

@hainest hainest closed this as completed Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hainest hainest

Labels
API-BREAKER This change breaks the public API BPatch This issue is directly related to BPatch code cleanup Bring the code up to modern standards or remove deprecated features
Projects
None yet
Milestone
Release 11.0
Development

Successfully merging a pull request may close this issue.

Remove BPatch_addressSpace::findFunctionByAddr
2 participants
@mxz297 @hainest