On 10/20/2011 08:43 PM, Eli Zaretskii wrote:
>> Date: Thu, 20 Oct 2011 20:19:33 +0200
>> From: Sebastian Pipping <[email protected]>
>>
>> I have some new code ready for you to tear apart, err review :-)
>
> Here's mine:
Thanks Eli!
> + /* Determine target file (i.e. stdout or stderr) and color to pick */
> + switch (type)
> + {
> + case OT_DIR_ENTER: target = stdout; color = color_dir_enter; break;
> + case OT_DIR_LEAVE: target = stdout; color = color_dir_leave; break;
> + case OT_MISC_MESSAGE: target = stdout; color = color_misc_message;
> break;
> + case OT_MISC_ERROR: target = stderr; color = color_misc_error; break;
> + case OT_MISC_FATAL: target = stderr; color = color_misc_fatal; break;
> + case OT_EXECUTION: target = stdout; color = color_execution; break;
> + default: target = stdout; color = ""; colorize = 0; break;
> + }
>
> This should probably be a data structure, indexed by the OT_* values.
Actually, I asked myself if I should go for a table lookup but I somehow
decided against it. (I was wondering if putting "stderr" into a stacic
map would end up with a fully initialized stderr or if there could be
races on that. If there can, I would need an extra function to get the
current value of stderr during runtime. I was also worried if C90 would
allow me to build that table as I wanted. That's where the doubts pot
flow over... an made me go to switch.)
So while I don't obect to such a change: what kind of gain are you
aiming here? Speed? Maintainability? Would be cool to hear more.
Best,
Sebastian
_______________________________________________
Bug-make mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/bug-make
