Re: [dev] [PATCH] Add RGB color definition

From: Alexander Sedov <alex0player_AT_gmail.com>
Date: Sat, 20 Jul 2013 23:26:50 +0400

2013/7/19 Roberto E. Vargas Caballero <k0ga_AT_shike2.com>:
> From: "Roberto E. Vargas Caballero" <k0ga_AT_shike2.com>
>
> It is impossible allocate all the RGB colors using a static
> array, so it is necessary move DC.col into a pointer and use
> dinamic memory.
>
> Since the color definition is not used to much is not a bad idea
> use realloc each time. It means the color definition arry will
> increase its size until user resets the terminal.
>
> If this sequence is used a lot we should check if the color
> is already defined and look for a way ef releasing colors
> not used (and maybe a better allocation scheme).
I disapprove of this patch, since it does not work the way advertised.
Particularly, added colors can be palette-swapped with another
sequence. Also, replacing static memory with malloc() feels dirty.
You also turn if () {...} to if () ... for absolutely no reason, even
though it's quite against the guidelines.
What I'd suggest is using special colour indices (like 0x1RRGGBB, 1 is
for ensuring large numbers) for true colors, and wrapping colour
access in xdraws(). Much cleaner, doesn't touch heap.
Received on Sat Jul 20 2013 - 21:26:50 CEST