[Home]

Summary:ASTERISK-24817: init_logger_chain: unreachable code block
Reporter:Corey Farrell (coreyfarrell)Labels:
Date Opened:2015-02-21 13:11:26.000-0600Date Closed:2015-03-19 05:20:25
Priority:MinorRegression?
Status:Closed/CompleteComponents:Core/Logging
Versions:SVN 11.16.0 13.2.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) logger-unreachable.patch
Description:init_logger_chain returns immediately if logger.conf was not found or for parse errors.  Later the procedure has an unreachable block that would set default console logging if no config was found.

In 11 and 13 I think the console logger should be applied if logger.conf is missing or invalid during startup only.  During failed reload we should just return as it does now.
In trunk I think this should be a fatal error during startup, and the unreachable code should be removed.  Failed reload's should continue to be a no-op.
Comments:By: Matt Jordan (mjordan) 2015-02-21 16:53:45.954-0600

I'm not sure we should remove the unreachable code in trunk. The act of providing a default console logger if no {{logger.conf}} is available was probably always the intent.

By: Rusty Newton (rnewton) 2015-02-25 08:52:09.396-0600

{quote}In trunk I think this should be a fatal error during startup, and the unreachable code should be removed.{quote}

Corey why do you feel this way?

By: Corey Farrell (coreyfarrell) 2015-02-25 10:16:36.565-0600

Just does not seem like Asterisk should run without real logging (to a file) unless the admin explicitly sets it up that way (with a valid logger.conf).  I don't feel that strongly about this.  I posted here to allow someone to disagree with me, and Matt has.  That's good enough.

The patch I have is attached.  I don't have a chance to test this now (beyond knowing it compiles).  Hopefully I'll get around to testing next week.

By: Rusty Newton (rnewton) 2015-02-25 14:11:17.235-0600

Cool. I was curious. I could go either way on it. If we end up loading a console logger by default when there is no config then I think there should definitely be some clear warnings on the console stating that.



By: Corey Farrell (coreyfarrell) 2015-03-14 16:26:36.632-0500

I've thought of this more.  If we're going to apply default console logger when logger.conf fails at startup, then we also need to replace existing logger channels during reload.  This is needed since the reload process is expected to close and reopen logger channels on the assumption that log files were rotated.  If we're going to deal with missing logger.conf at startup, we have to deal with situations where someone had a logger.conf at startup, then intentionally deleted it.