First installment (just the #defines):
(#define): Add _GNU_SOURCE.
Jan: do not understand what this #define means.
It enables:
STRICT_ANSI, _ISOC99_SOURCE, _POSIX_SOURCE, _POSIX_C_SOURCE,
_XOPEN_SOURCE, _XOPEN_SOURCE_EXTENDED, _LARGEFILE_SOURCE,
_LARGEFILE64_SOURCE, _FILE_OFFSET_BITS=N, _BSD_SOURCE, _SVID_SOURCE
but I swapped it out for:
#define _XOPEN_SOURCE 700
since that is supposed to give better portability. Without either of
those defines, my compiler complains a lot about implicit fucntion
definitions. However, that said, it will still link and run.
Thoughts?
Re complexity:
Jan: If they are not needed, they should be removed, and if they are
needed then there is a serious complexity problem.
I double checked and added comments for what all those includes are.
Seems I cannot reduce the number further.
#include <assert.h> // assert in log_msg()
#include <libgen.h> // 'basename'
#include <stdarg.h> // va_args
#include <stdio.h>
#include <stdlib.h> // abort() and basename
#include <string.h>
#include <time.h> // time ops
#include <unistd.h> // for unlink (symlink)
#include <errno.h> // errorno for file opening
#include <dirent.h> // dir operations
#include <sys/stat.h> // req for lstat (symlink)
#include <fcntl.h> // needed for file ops
Regards the symlink:
Jan: This all sound good, except the symlink, that is not going to work in
windows or IOS.
Can we put a compiler directive here since for unix systems, having
this service is actually very useful and saves a lot of user/dev time?
More to come soon,
G
On Sun, Aug 23, 2015 at 11:00 AM, jan i <[email protected]> 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.
--
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
