corinthia-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jan i <>
Subject Fwd: [1/2] incubator-corinthia git commit: Logging feature update
Date Sun, 23 Aug 2015 10:00:53 GMT
Hi Gabriela

Finally I got time to go in detail with your work. First thanks for making
this important work.

I have some comments to your latest commit:

---------- Forwarded message ----------

"Add dedicated general logging macros, set a symlink 'current.log', and
allow local custom log macros with dedicated names for the log file.
Remove the tar file and use snprintf instead strncat and strdup."

This all sound good, except the symlink, that is not going to work in
windows or IOS.

    (#define): Add _GNU_SOURCE.
do not understand what this #define means.

    (#include): Add "log_maker.h".
                Add <assert.h>.
                Add <error.h>.
                Add <libgen.h>.
                Add <stdarg.h>.
                Add <stdio.h>.
                Add <stdlib.h>.
                Add <string.h>.
                Add <time.h>.
                Add <unistd.h>.
                Add <errno.h>.
                Add <dirent.h>.
                Add <sys/types.h>.
                Add <sys/stat.h>.
                Add <fcntl.h>.
      These are probably not all needed.
If they are not needed, they should be removed, and if they are needed then
there is a serious complexity problem.

    (get_time_string): New function.

    (set_log_output_function): New function.  This is a stub.
I thought we agreed to make that part of log_init, I do not like it as an
extra function.

    (log_init): New function.  Create the logfile name with host, time
      and program name. Set a symlink 'current.log' into
the log_init function is given an output_function, it should NOT make any
file operations.

You should add a default_output_function, which you use if you get a NULL
pointer in the log_init call.

    (set_log_dir): New function.  Set the directory in which to write
      the individual logfiles and the location where the symlonk
      should be created.
that is part of the output_function. Forget symlink, the program runs in a
work-dir, and that is where the log files should go.

    (close_log): New function.  Close the file descriptor and inform
      user about the location and name of the generated file.
I dont like this function, it adds complexity to the code, what happens if
I call close_log, and then log a message.

It is really not needed, close is done automatially when the program
closes. In the output function you should use fflush() to secure
all buffers are written to disk.

    (log_msg_prefix): New function.  Create the prefix containing the
      name of the log level, the file, line number and function name.
I would assume that is part of the log_msg call, I do not like to see 2
different calls.

    (log_msg): New function. Write the log prefix and log message to

  * log_maker.h:

    Header file for general inclusion so log_maker.c can be used.

    (#ifndef): Add LOG_MAKER_H.
      Header guard.  Somehow #pragma once does not work for me.
Look at our other headers, there #pragma seems to work fine.

    (#if): Add _MSC_VER.  Picked this up in the docs, apparently MSC
      uses _sprintf instead.

this is something that must go in dfplatform.h, we do not want to have MSC
dependencies throughout the code.

    (#define):  Add LOG_ERR.
                Add LOG_WARNING.
                Add LOG_NOTICE.
                Add LOG_INFO.
                Add LOG_DEBUG.
        These are all strings for now.  I can change it back to numbers, but
        currently those strings are used to print out the log message

    Global variables:

    (char): Add log_filename[filename_len].
that is the output_function and as such not the log code. If the output
function uses a file, code outside logger must open that file.

    (int): Add log_file_fd.  File descriptor for log file.
Again output_function.

    (static int log_level_initialised): Guard to prevent log_init from
      being called twise.

    (char): Add logging_dir[filename_len].

    (char): Add log_symlink[filename_len].

    (LOG_THIS): New function.  Basic macro, called by every custom macro.

    Basic log macros for general usage:

    (LOG_ERROR_MSG): New function.

    (LOG_WARNING_MSG): New function.

    (LOG_NOTICE_MSG): New function.

    (LOG_INFO_MSG): New function.

    (LOG_DEBUG_MSG): New function.

    Prototypes for logmaker.c:

    (set_log_output_function): New function.
no part of log_init

    (log_init): New function.

    (set_log_dir): New function.
no not used at all

    (set_log_level): New function.
no part of log_init

    (close_log): New function.

    (log_msg_prefix): New function.
I think this is part of log_msg

Can we agree that on the outside logger has:

log_init(<level>, <output_function> maybe other setup parms)
log_msg(..) the function the macros call.

LOG_FOO_MSG("text", ...)

Not more!

if log_init() is called with a NULL pointer for output_function, you use
your own, that logs fixed to corinthia.log in currenct directory.

jan i.

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message