[PATCH V6 08/13] cmd: bmp: Split bmp commands and functions

[Nikhil M Jain]

On 06/04/23 13:11, Vignesh Raghavendra wrote:
> Hi Nikhil,
> 
> On 06/04/23 11:57, Nikhil M Jain wrote:
>>>> +struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>>>> +                            void **alloc_addr)
>>>> +{
>>>> +       void *dst;
>>>> +       unsigned long len;
>>>> +       struct bmp_image *bmp;
>>>> +
>>>
>>> if (!IS_ENABLED(CONFIG_VIDEO_BMP_GZIP))
>>>      return NULL;
>>>
>>
>> I preferred to use #if to avoid compilation of code when not required.
>>
>> For example,  if someone doesn't want to display a gzip bmp image they
>> wouldn't want the code to be compiled, so that binary size doesn't
>> increase.
> 
> Both are equivalent. Compiler will optimize out the function if
> CONFIG_VIDEO_BMP_GZIP is not defined.
> 
> #ifdefs are complicated to read compared to inline if()s (at least for me).
> 
>>
>>>> +       /*
>>>> +        * Decompress bmp image
>>>> +        */
>>>> +       len = CONFIG_VIDEO_LOGO_MAX_SIZE;
>>>> +       /* allocate extra 3 bytes for 32-bit-aligned-address + 2
>>>> alignment */
>>>> +       dst = malloc(CONFIG_VIDEO_LOGO_MAX_SIZE + 3);
>>>> +       if (!dst) {
>>>> +               puts("Error: malloc in gunzip failed!\n");
>>>> +               return NULL;
>>>> +       }
>>>> +
>>>> +       /* align to 32-bit-aligned-address + 2 */
>>>> +       bmp = dst + 2;
>>>> +
>>>> +       if (gunzip(bmp, CONFIG_VIDEO_LOGO_MAX_SIZE, map_sysmem(addr, 0),
>>>> +                  &len)) {
>>>> +               free(dst);
>>>> +               return NULL;
>>>> +       }
>>>> +       if (len == CONFIG_VIDEO_LOGO_MAX_SIZE)
>>>> +               puts("Image could be truncated (increase
>>>> CONFIG_VIDEO_LOGO_MAX_SIZE)!\n");
>>>> +
>>>> +       /*
>>>> +        * Check for bmp mark 'BM'
>>>> +        */
>>>> +       if (!((bmp->header.signature[0] == 'B') &&
>>>> +             (bmp->header.signature[1] == 'M'))) {
>>>> +               free(dst);
>>>> +               return NULL;
>>>> +       }
>>>> +
>>>> +       debug("Gzipped BMP image detected!\n");
>>>> +
>>>> +       *alloc_addr = dst;
>>>> +       return bmp;
>>>> +}
>>>> +#else
>>>> +struct bmp_image *gunzip_bmp(unsigned long addr, unsigned long *lenp,
>>>> +                            void **alloc_addr)
>>>> +{
>>>> +       return NULL;
>>>> +}
>>>> +#endif
>>>> +
> 
> [...]
> 

I will add a new patch which replaces #ifdef to if(CONFIG_IS_ENABLED()) 
and send V7 series.

Thank you,
Nikhil