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

bug in getEnv #2849

Closed
stress-tess opened this issue Nov 15, 2023 · 2 comments · Fixed by #2850
Closed

bug in getEnv #2849

stress-tess opened this issue Nov 15, 2023 · 2 comments · Fixed by #2850
Assignees
@stress-tess
Labels
bug Something isn't working

Comments

@stress-tess
Copy link
Member

This was brought to my attention by @hokiegeek2

My fix to getEnv in #2817 left the cast to string (:string) since this is what the old code did. But due to the c_string deprecation this was returning a string of the pointer instead

So instead of getting something like "MY_ENV_VAL" I would get "0x16b167e28"

After finding the below link, I found that I need to use string.createCopyingBuffer instead
https://chapel-lang.org/docs/language/evolution.html#c-string-deprecation
@stress-tess stress-tess self-assigned this Nov 15, 2023
@stress-tess stress-tess added the bug Something isn't working label Nov 15, 2023
stress-tess pushed a commit to stress-tess/arkouda that referenced this issue Nov 15, 2023
Fixes Bears-R-Us#2849: bug in getEnv
c011af8 
This PR (fixes Bears-R-Us#2849) since `c_string` has been deprecated we need to use `string.createCopyingBuffer` instead of casting
@stress-tess stress-tess mentioned this issue Nov 15, 2023
Merged
@hokiegeek2
Copy link
Contributor

@pierce314159 thanks so much for figuring this out!

github-merge-queue bot pushed a commit that referenced this issue Nov 15, 2023
@stress-tess
Fixes #2849: bug in getEnv (#2850)
f359644 
This PR (fixes #2849) since `c_string` has been deprecated we need to use `string.createCopyingBuffer` instead of casting

Co-authored-by: Pierce Hayes <[email protected]>
@hokiegeek2
Copy link
Contributor

@pierce314159 @bmcdonald3 confirmed getEnv is working correctly w/ Arkouda built w/ Chapel 1.32:

image

Thanks to you both for making this fix happen! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stress-tess stress-tess

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #2849: bug in getEnv stress-tess/arkouda
2 participants
@hokiegeek2 @stress-tess