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

[Vignesh Raghavendra <vigneshr@ti.com>]

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
>>> + 

[...]

-- 
Regards
Vignesh