[Home]

Summary:ASTERISK-03839: [patch] Remove coloring escape sequences from file/syslog logs
Reporter:Paul Cadach (pcadach)Labels:
Date Opened:2005-04-01 15:10:24.000-0600Date Closed:2008-01-15 15:30:36.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) logger-nocolor.diff
Description:Very simple but makes log files much more readable.

****** ADDITIONAL INFORMATION ******

Disclaimer is on file.
Comments:By: Kevin P. Fleming (kpfleming) 2005-04-01 15:40:46.000-0600

I don't think there's any need to cast NULL to a 'char *'.

Wouldn't it just be more efficient to replace the escape sequences with spaces instead of shrinking the string down (potentially multiple times)?
By: Kevin P. Fleming (kpfleming) 2005-04-01 15:41:41.000-0600

Actually, the compiler is going to optimize away an '!= NULL' check anyway, it's redundant :-)
By: Paul Cadach (pcadach) 2005-04-01 16:24:50.000-0600

Replacing escapes with spaces isn't a case because it will totally destroy any formatting and make logs hard-readable like with escape sequences.
By: Kevin P. Fleming (kpfleming) 2005-04-01 16:33:55.000-0600

OK, that makes sense... there is no other character that won't take up space. Do you have any idea as to the performance impact of this change?
By: Paul Cadach (pcadach) 2005-04-01 16:43:30.000-0600

Copying from one string pointer by another would have more worst preformance IMHO because:
1) strchr() (just read) have more optimal implementation than read and write;
2) many strings have to go without formatting so make a copy of it is waste of CPU power.
By: Kevin P. Fleming (kpfleming) 2005-04-01 17:57:01.000-0600

Committed to CVS, with strip_coloring reimplemented to not copy the same characters more than one time. Thanks!
By: Tilghman Lesher (tilghman) 2005-04-01 22:45:51.000-0600

I think this bug probably should have used the term_strip routine already in term.c, instead of implementing a whole new subroutine in an unrelated module.
By: Kevin P. Fleming (kpfleming) 2005-04-01 23:12:08.000-0600

Yes, it could have, except that function requires using two separate buffers, which would be wasteful for this situation.
By: Paul Cadach (pcadach) 2005-04-01 23:49:57.000-0600

Corydon76, removing coloring on logs, not on terminal. Because function declared as static it's includes into calling code by compiler optimizations (saves call/return), so better is to have functions in the same source where it referenced if it called locally (or have specific usage). Also, check my previous comment to not use copying of characters if all work done within the same buffer.

kpfleming: Usage of strchr is more optimal than the same done by hand. It have inline implementation when compiled with optimizations.
By: Kevin P. Fleming (kpfleming) 2005-04-02 17:21:03.000-0600

Yes, you are right, there was no reason to open-code the first strchr in there. I'll fix that. The second one is open-coded because it actually copies the characters it's passing over.
By: Russell Bryant (russell) 2005-04-05 01:16:52

all changes added to 1.0 as well
By: Digium Subversion (svnbot) 2008-01-15 15:30:01.000-0600

Repository: asterisk
Revision: 5354

U   trunk/logger.c

------------------------------------------------------------------------
r5354 | kpfleming | 2008-01-15 15:30:01 -0600 (Tue, 15 Jan 2008) | 2 lines

strip color code sequences from verbose messages being sent to files/syslog (bug ASTERISK-3839, with mods)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5354
By: Digium Subversion (svnbot) 2008-01-15 15:30:36.000-0600

Repository: asterisk
Revision: 5394

U   branches/v1-0/CHANGES
U   branches/v1-0/logger.c

------------------------------------------------------------------------
r5394 | russell | 2008-01-15 15:30:36 -0600 (Tue, 15 Jan 2008) | 2 lines

merge kevin's patches to remove color escape sequences from the logs (bug ASTERISK-3839)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5394