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

From: Roberto E. Vargas Caballero <>
Date: Sat, 20 Jul 2013 21:37:04 +0200

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.

I don't see how you can swapped them, and I agree with you that the
malloc can be a big problem (I already said in the commit message). It
was only a first version and I was waiting people could say his opinion
and a better way of doing it.

> You also turn if () {...} to if () ... for absolutely no reason, even
> though it's quite against the guidelines.

I did because I thought the guidelines was the inverse, use {} only if
the body is bigger than 1 line, in other case don't use it. The problem
here is the guidelines are not written, so I think we should listen
what have to say Christoph.

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

I agree with you, your solution is far better than mine, it removes
the malloc and I think it will take less code. I will try implement
it this night.

Thank you.

Roberto E. Vargas Caballero
Received on Sat Jul 20 2013 - 21:37:04 CEST

This archive was generated by hypermail 2.3.0 : Sat Jul 20 2013 - 21:48:05 CEST