Just a follow up, I see
log_msg(char* level, char* filename, int linenum, char *msg, ...)
{
if (level active)
{
snprintf(buffer, ".......", char* filename, int linenum, char *msg,
...)
call output_function(buffer)
}
}
I hope this explains a bit better how I think.
rgds
jan I.
On 23 August 2015 at 12:00, jan i <j...@apache.org> wrote:
> 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
> ~/../incubator/.
> 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
> file.
>
> * 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
> titles.
>
> 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].
> no.
>
> (char): Add log_symlink[filename_len].
> no.
>
> (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.
> no
>
> (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.
>
> rgds
> jan i.
>
