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

RFC: Deprecate r_str_new and r_str_dup #20959

Closed
4 tasks done
trufae opened this issue Oct 28, 2022 · 6 comments
Closed
4 tasks done

RFC: Deprecate r_str_new and r_str_dup #20959

trufae opened this issue Oct 28, 2022 · 6 comments

Comments

@trufae
Copy link
Collaborator

trufae commented Oct 28, 2022

  • r_str_dup
  • r_str_new
  • R_STR_DUP
  • r_str_ndup / r_str_newlen

It can be tricky for rmalloc as strdup uses libc's malloc and r_str could use r_malloc alternative, causing inconsistencies between new/free and overcomplicates the logic. so maybe we can just move forward and just use strdup and deprecate those.

About r_str_new that could be kept for consistency with r_str_newf()

strdup() is available everywhere and works the same.
Screenshot 2022-10-29 at 01 10 13

@trufae
Copy link
Collaborator Author

trufae commented Mar 15, 2024

its anoying to my eyes. but not really a blocker, moving forward

@Ashishbsharma
Copy link

Hi, I'd like to contribute to this issue, please guide me where to start.

@trufae
Copy link
Collaborator Author

trufae commented Apr 25, 2024

As long as this is an abi breaking change this can't be done until we reach 5.9.9.

What can be done before:

  • remove the use of all those calls to use only strdup()
  • decide/discuss if its needed to have a nullchk version (as a macro) R_STRDUP (or R_STR_DUP?)

What can't be done:

  • remove the public functions

@trufae
Copy link
Collaborator Author

trufae commented Apr 25, 2024

Feel free to join the discord for further discussions

@satk0
Copy link
Contributor

satk0 commented Jun 28, 2024

Hey, is this issue still not resolved? I can take it if you don't mind.

IMHO, being more careful is more important than being less, so I would opt for creating a new R_STRDUP nullcheck macro and replace all R_STR_DUP occurences with it to avoid any old api confusions.

As far as I understand, the r_str_ndup / r_str_newlen should be replaced with strndup, right?

Moreover, I've found some syntax mistakes with invoking strdup (strdup( instead of strdup (). I could correct it too.

satk0 added a commit to satk0/radare2 that referenced this issue Jul 12, 2024
satk0 added a commit to satk0/radare2 that referenced this issue Jul 12, 2024
satk0 added a commit to satk0/radare2 that referenced this issue Jul 12, 2024
satk0 added a commit to satk0/radare2 that referenced this issue Jul 12, 2024
satk0 added a commit to satk0/radare2 that referenced this issue Jul 12, 2024
satk0 added a commit to satk0/radare2 that referenced this issue Jul 13, 2024
satk0 added a commit to satk0/radare2 that referenced this issue Jul 22, 2024
@trufae trufae modified the milestones: 6.0.0, 5.9.4 - icecore Aug 5, 2024
@trufae
Copy link
Collaborator Author

trufae commented Aug 5, 2024

thank you @satk0 !!

@trufae trufae closed this as completed Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants