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

iStats #86: CFStringGetCStringPtr usually returns null. #88

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zenenwjaimes
Copy link

CFStringGetCStringPtr usually returns null. I added a fallback with CFStringGetCString to allocate a string. This is consistent and in compliance with Apple. The following link should give a bit more insight.

CFStringGetCStringPtr Obj-C doc

With this fallback type of system, older systems will still behave as they did and newer ones will get the battery string the new way. Either way, it should all work the same. The only thing I'm unclear of and maybe you can help with, is how does the freeing of the pointer work? I'm not familiar with rb_str_new2, so I don't know if the GC will clean up that pointer or not.

If there's any improvements I can make or you can shed more info on how this works, that would be great. Thanks for this great little tool, you saved me plenty of time reinventing the wheel.

p.s. Can you also give a bit more info as to how to better/quicker debug? Thanks again!

… fallback

with CFStringGetCString. This is consistent and in compliance with Apple
documentation.
@Chris911
Copy link
Owner

Thanks for your contribution @zenenwjaimes. Here's the command I usually use to test changes to the C files. It builds the bundle and executes iStats. Let me know if it works for you.

cd ext/osx_stats && ruby extconf.rb && make && cd - && cp ext/osx_stats/osx_stats.bundle lib && ./bin/istats

@zenenwjaimes
Copy link
Author

Thanks @Chris911! Github has horrible notifications so I apologize for getting back to you so late. Keep up the good work, I'll try and see if there's any other bugs I can tackle. Thanks again.

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.

2 participants