[Libva] [PATCH] android-log: enable logs according to Android version

[Charles, Daniel]

Hi

On Tue, Jul 31, 2012 at 9:08 PM, Gwenole Beauchesne <gb.devel at gmail.com> wrote:
> Hi,
>
> 2012/7/31 Daniel Charles <daniel.charles at intel.com>:
>
>> diff --git a/va/sysdeps.h b/va/sysdeps.h
>> index 0752b17..5f1c6a9 100644
>> --- a/va/sysdeps.h
>> +++ b/va/sysdeps.h
>> @@ -22,11 +22,11 @@
>>   * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>   */
>>
>> -#ifndef SYSDEPS_H
>> -#define SYSDEPS_H
>> +#ifndef LIBVA_SYSDEPS_H_
>> +#define LIBVA_SYSDEPS_H_
>>
>>  #ifdef HAVE_CONFIG_H
>> -# include "config.h"
>> +#include "config.h"
>>  #endif
>>
>>  #include <stdio.h>
>
> No need to change anything in this hunk.

ok, I guess you want to keep indentation here.
>
>> @@ -36,9 +36,27 @@
>>  #include <assert.h>
>>
>>  #ifdef ANDROID
>> -# define Bool  int
>> -# define True  1
>> -# define False 0
>> +#define Bool  int
>> +#define True  1
>> +#define False 0
>
> Same here.

Same guess as above.

>
>> +#define LOG_TAG "lib-va"
>> +#include <utils/Log.h>
>> +
>> +#ifdef ANDROID_ALOG
>> +#define VA_LOGE(buffer) ALOGE("%s", buffer);
>
> va_log_error()

ok, to be changed

>
>> +#define VA_LOGI(buffer) ALOGI("%s", buffer);
>
> va_log_info()
>
ditto

>> +#define VA_LOGV(buffer) ALOGV("%s", buffer);
>
> Where is this used? If not used, just drop it; or name it va_log_verbose()?
>
to be removed

>> +#elif ANDROID_LOG
>> +#define VA_LOGE(buffer) LOGE("%s", buffer);
>> +#define VA_LOGI(buffer) LOGI("%s", buffer);
>> +#define VA_LOGV(buffer) LOGV("%s", buffer);
>>  #endif
>
> Likewise.
>
ok

>> +#endif//LIBVA_SYSDEPS_H_
>
> Why is this changed? It's not an installed header.
>
to make it less generic, but it can remain as before.

>> diff --git a/va/va.c b/va/va.c
>> index 1bbe047..6adf821 100644
>> --- a/va/va.c
>> +++ b/va/va.c
>> @@ -105,21 +105,24 @@ int vaDisplayIsValid(VADisplay dpy)
>>
>>  void va_errorMessage(const char *msg, ...)
>>  {
>> +    char buffer[1024];
>>      va_list args;
>>
>> -    fprintf(stderr, "libva error: ");
>>      va_start(args, msg);
>> -    vfprintf(stderr, msg, args);
>> +    vsnprintf(buffer, 1024, msg, args);
>
> Several errors: use sizeof, and check return value. If undersized,
> allocate a new buffer.
> Same for va_infoMessage().

Someone recommended offline to use vasnprintf, it will avoid check
plus re-allocate, the chances to re-size are really high here.

I will submit a new version during my day time tomorrow.

Thanks for your review

-- 
Daniel.
>
> Thanks,
> Gwenole.