Hi Gabriela, this looks like a good start. A few suggestions come to mind:
- I recommend defining a macro for each of the log levels, each of which invokes LOG_THIS. Having these separate macros (LOG_ERROR_MSG, LOG_WARNING_MSG, etc) will mean that for production builds we can conditionally compile such that these macros do nothing, which will result in faster execution. When debugging, they will be enabled so we can see the messages. For production build we may want to show only errors, but probably hide warnings, info etc. - Later on, it might be worth us having macros or log parameters for different modules. For example when debugging the XML parser or hash table functions, it’ll be best to hide (possibly verbose and likely irrelevant) log output from other components. Thus for a test, someone might be converting .docx to .html, but only want to see messages relating to DFHashTable. - I’d advise against strcat and even strncat, as well as using malloc with the sum of the length of strings you want to concatenate - this is very easy to get wrong. A simple solution is to allocate a sufficiently large string (e.g. 1024 characters) and then use snprintf. Alternatively, DFFormatString will do the necessary work for you of allocating enough memory and constructing a string with the specified parameters. I realise your code doesn’t currently use DocFormats, but you could take DFFormatString and copy it into your code, adapting it as necessary. - Rather than placing the tar file in the repository as a single file, just extract it and have the individual files commited. This way we can track changes and easily make patches as we do with the other code, rather than having to continuously update the tar file each time. — Dr Peter M. Kelly pmke...@apache.org PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key> (fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966) > On 18 Aug 2015, at 4:30 am, Gabriela Gibson <gabriela.gib...@gmail.com> wrote: > > Hi, > > Jan kindly tasked me with making a logger for Corinthia and helped me > figure out what it needed initially. > > Currently it only produces a log file and uses a small combination of > macros to write out the file, line and function. It will acquire function > pointers for users to hook in their own output functions and other stuff > in the next iteration. > > However, whilst this still misses a lot of the spec, this is a good > time to take a look to see how this is all going and if what I > concocted here is portable, or even a good idea. > > Caveat: I have no clue why I keep getting the complaints from gcc, > it does compile, link and run in the end.[1] > > You can find it here: > > https://github.com/apache/incubator-corinthia/blob/master/experiments/logger/log_maker.tar > > Files: > > log_maker.h // header > log_maker.c // code for the log mechanism > useLogmaker.c // test code > Makefile > > You probably want to set the log directory to something useful, it's > currently set to tmp/foo/bar, in useLogmaker.c main: 25 in the line > set_log_dir("/tmp/foo/bar/"); > > to build and run, it's > > $ make; ./logMaker > > After the program is completed, it shows a message giving the path and > name of the created log file. > > G > > [1] Output on my machine: > > gcc -ggdb -std=c99 -Wall -c -o useLogmaker.o useLogmaker.c > In file included from useLogmaker.c:1:0: > log_maker.h:26:12: warning: ‘global_log_level’ defined but not used > [-Wunused-variable] > static int global_log_level = LOG_WARNING; > ^ > log_maker.h:28:12: warning: ‘log_level_initialised’ defined but not > used [-Wunused-variable] > static int log_level_initialised = 0; > ^ > gcc -ggdb -std=c99 -Wall -c -o log_maker.o log_maker.c > log_maker.c: In function ‘log_msg_prefix’: > log_maker.c:183:5: warning: implicit declaration of function ‘dprintf’ > [-Wimplicit-function-declaration] > dprintf(log_file_fd, "%s %s %s:%d %s() ", level_prefixes[level], > time_buf, filename, linenum, function); > ^ > log_maker.c: In function ‘log_msg’: > log_maker.c:194:5: warning: implicit declaration of function > ‘vdprintf’ [-Wimplicit-function-declaration] > vdprintf(log_file_fd, fmt, argp); > ^ > gcc -ggdb -std=c99 -Wall -o logMaker useLogmaker.o log_maker.o > Logfile created in /tmp/foo/bar/logMaker.musashi.20150813-182555 > > > -- > Visit my Coding Diary: http://gabriela-gibson.blogspot.com/
