Attention is currently required from: neels.

View Change

6 comments:

• File include/osmocom/core/logging.h:

  • Patch Set #1, Line 400: /* Set timezone for logging of timestamps: 0 for local time, 1 for UTC. Other values are reserved for future

    0 and 1? don't we have enums for that in this patch?

• File src/core/logging.c:

  • Patch Set #1, Line 474: return 0;

    I'd really return an error if the requested timestamp cannot be obtained.

  • Patch Set #1, Line 528: } else if (target->print_timestamp && get_timestamp(&tv, &tm, target) == 0) {

    you are calling get_timestamp twice here every time we log something. You can move get_timestamp in front and call it once.

  • Patch Set #1, Line 874: target->timezone = timezone;

    this should return an int and return an error if user wants to set a timezone for which we don't have support (#ifdef HAVE_LOCALTIME_R for instance).

• File src/vty/logging_vty.c:

  • Patch Set #1, Line 203: log_set_timezone(tgt, LOG_TIMEZONE_UTC);

    in here, chewck the error code returned and if it failed return CMD_WARNING with a logging saying that this timezone is not supported in this system.

• File tests/logging/logging_vty_test.vty:

  • Patch Set #1, Line 75: logging_vty_test# logging timestamp ?

    you could add a "logging timezone ?" test here.

To view, visit change 32043. To unsubscribe, or for help writing mail filters, visit settings.