Re: [Qemu-devel] [PATCH] audio: Drop superfluous conditionals around g_free()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 06/06/2014 10:35 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  audio/alsaaudio.c      | 12 ++++--------
>>  audio/audio_template.h | 15 ++++-----------
>>  audio/ossaudio.c       |  6 ++----
>>  hw/audio/adlib.c       |  4 +---
>>  4 files changed, 11 insertions(+), 26 deletions(-)
>> 
>
>> +++ b/audio/audio_template.h
>> @@ -71,10 +71,7 @@ static void glue (audio_init_nb_voices_, TYPE) (struct 
>> audio_driver *drv)
>>  
>>  static void glue (audio_pcm_hw_free_resources_, TYPE) (HW *hw)
>>  {
>> -    if (HWBUF) {
>> -        g_free (HWBUF);
>> -    }
>> -
>> +    g_free (HWBUF);
>
> While touching this, you may want to drop the space.
>
>> @@ -92,9 +89,7 @@ static int glue (audio_pcm_hw_alloc_resources_,
>> TYPE) (HW *hw)
>>  
>>  static void glue (audio_pcm_sw_free_resources_, TYPE) (SW *sw)
>>  {
>> -    if (sw->buf) {
>> -        g_free (sw->buf);
>> -    }
>> +    g_free (sw->buf);
>
> here too.
>
>> @@ -172,10 +167,8 @@ static int glue (audio_pcm_sw_init_, TYPE) (
>>  static void glue (audio_pcm_sw_fini_, TYPE) (SW *sw)
>>  {
>>      glue (audio_pcm_sw_free_resources_, TYPE) (sw);
>> -    if (sw->name) {
>> -        g_free (sw->name);
>> -        sw->name = NULL;
>> -    }
>> +    g_free (sw->name);
>> +    sw->name = NULL;
>
> hmm, it looks like that style is pervasive in this file.

All files are equal in coding style, but some files are more equal than
others.

Specifically, audio/ and hw/audio/.

>> +++ b/audio/ossaudio.c
>> @@ -736,10 +736,8 @@ static void oss_fini_in (HWVoiceIn *hw)
>>  
>>      oss_anal_close (&oss->fd);
>>  
>> -    if (oss->pcm_buf) {
>> -        g_free (oss->pcm_buf);
>> -        oss->pcm_buf = NULL;
>> -    }
>> +    g_free(oss->pcm_buf);
>> +    oss->pcm_buf = NULL;
>
> Here, you DID trim the space.  Be consistent - either preserve the
> original spacing, or clean it up everywhere.

I meant to stick to the audio style, but fingers overruled brain here.

> At any rate, whitespace is trivial, so either way:
>
> Reviewed-by: Eric Blake <address@hidden>

Gerd, should patches for audio/ conform to the usual style now, or are
they still special?