First, thanks for looking after OSX :-)
And then - could you try sending your patch with git send-email (or git
format-patch and then attach the resulting file)? This looks weird, with
half of the patch in an attachment and the header outside the attachment.
On 2015-04-19 08:57, Mihai Moldovan wrote:
>
> diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c
> index cb62661..ad5c07b 100644
> --- a/src/modules/macosx/module-coreaudio-device.c
> +++ b/src/modules/macosx/module-coreaudio-device.c
> @@ -375,6 +375,39 @@ static int ca_sink_set_state(pa_sink *s, pa_sink_state_t state) {
> return 0;
> }
>
> +/* Caveat: this function frees the CFString if conversion succeeded. */
I'm not actually following the reasoning. Is the CFString leaked if the
conversion fails, that can't be a good thing?
> +static int CFString_to_cstr_n(CFStringRef cfstr, char *buf, long n) {
Maybe return a bool here instead of int? Often an int result of 0 means
success/no error, here's it's the opposite, which is confusing.
> + int ret;
> +
> + assert (buf);
> +
> + ret = 0;
> +
> + if (cfstr != NULL) {
> + const char *tmp = CFStringGetCStringPtr(cfstr, kCFStringEncodingUTF8);
> +
> + if (tmp == NULL) {
> + if (CFStringGetCString(cfstr, buf, n, kCFStringEncodingUTF8))
> + ret = 1;
> + }
> + else {
> + strncpy(buf, tmp, n);
> + buf[n - 1] = 0;
> + ret = 1;
> + }
> + }
> +
> + /*
> + * A true value for ret implies cfstr != NULL, but let's still do the check
> + * for safety reasons (i.e., should this code ever be re-organized...)
> + */
> + if (ret && cfstr != NULL) {
> + CFRelease(cfstr);
> + }
Can't you just move it a line above, inside the "if (cfstr != NULL)"
part? Then you don't need to check for it again.
> +
> + return ret;
> +}
> +
> static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx) {
> OSStatus err;
> UInt32 size;
> @@ -387,6 +420,7 @@ static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx
> char tmp[255];
> pa_strbuf *strbuf;
> AudioObjectPropertyAddress property_address;
> + CFStringRef tmp_cfstr;
>
> ca_sink = pa_xnew0(coreaudio_sink, 1);
> ca_sink->map.channels = buf->mNumberChannels;
> @@ -401,7 +435,11 @@ static int ca_device_create_sink(pa_module *m, AudioBuffer *buf, int channel_idx
> property_address.mScope = kAudioDevicePropertyScopeOutput;
> property_address.mElement = channel_idx + i + 1;
> size = sizeof(tmp);
> - err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, tmp);
> + err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, &tmp_cfstr);
> + if (err == 0) {
> + err = !(CFString_to_cstr_n(tmp_cfstr, tmp, sizeof(tmp)));
> + }
> +
> if (err || !strlen(tmp))
> snprintf(tmp, sizeof(tmp), "Channel %d", (int) property_address.mElement);
>
> @@ -505,6 +543,7 @@ static int ca_device_create_source(pa_module *m, AudioBuffer *buf, int channel_i
> char tmp[255];
> pa_strbuf *strbuf;
> AudioObjectPropertyAddress property_address;
> + CFStringRef tmp_cfstr;
>
> ca_source = pa_xnew0(coreaudio_source, 1);
> ca_source->map.channels = buf->mNumberChannels;
> @@ -519,7 +558,11 @@ static int ca_device_create_source(pa_module *m, AudioBuffer *buf, int channel_i
> property_address.mScope = kAudioDevicePropertyScopeInput;
> property_address.mElement = channel_idx + i + 1;
> size = sizeof(tmp);
> - err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, tmp);
> + err = AudioObjectGetPropertyData(u->object_id, &property_address, 0, NULL, &size, &tmp_cfstr);
> + if (err == 0) {
> + err = !(CFString_to_cstr_n(tmp_cfstr, tmp, sizeof(tmp)));
> + }
> +
> if (err || !strlen(tmp))
> snprintf(tmp, sizeof(tmp), "Channel %d", (int) property_address.mElement);
>
>
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
