You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@corinthia.apache.org by Gabriela Gibson <ga...@gmail.com> on 2015/08/17 23:30:48 UTC

logger for Corinthia

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/

Re: logger for Corinthia

Posted by Gabriela Gibson <ga...@gmail.com>.
On Wed, Aug 19, 2015 at 4:51 PM, Peter Kelly <pm...@apache.org> wrote:
> Hi Gabriela, this looks like a good start.
>
Thanks, I was a bit worried that the macros would not be portable.
Then again, if we find that they are not, we can always underpin with
seperate code I guess (hope).

> 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.

Ok, I'll add that.  I initially had that but thought it would be
possible to turn off if I parameterize it.  Then again, if it ever
needs changing, a good find . -name ... macro will do the trick and
the dedicated macros also are a bit easier to read.

Out of (idle) interest: How much time are we actually saving on
execution though?

I'm asking in general, since we expect to run on handhelds.

Could we set an upper limit goal on execution time as a general target
to shoot for?

>
> - 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 had an idea like that, but it seemed a bit over complex  However, it
might be a good start for this topic and maybe there is a better way
of doing the same thing.  Or we can keep the simple macros and just
add another function later.

I've attached the original design, which I had pared down somewhat in
the initial presentation.  Jan thought it was a bit more than we need
for now (and I agree with him here)

I also thought better of the file name to use, the host machine is
useful to know and it may even be desirable to add user to that list.
Then again, I also have to say that the file name that is generated is
quite the eyesore, and some people might prefer it to simply be called
Corinthia.log.  So many ways of skinning a cat!

I still like the concept of the log groups in general, I often wish I
didn't have to comment out debug statements, and of course, every time
I delete them, I then find that I need them again.  Also, something
like that could be useful for adhoc profiling, I describe that in the
footnotes of the attached document.

Note that the design contains some elements that are not really needed
and some I've removed (for example, we don't need stuff like
LOG_EMERG), but I've left them in for now.

>
> - 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.

Ok, will do that for now and later when this all works, I can plug in
the DFormatString.

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.

Ok, will do that.  I keep the dedicated makefile for now, once this
all runs (instead of wobbling along :-) I'll plug it into the main
Corinthia tree.

G,

>
> —
> Dr Peter M. Kelly
> pmkelly@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 <ga...@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/
>



-- 
Visit my Coding Diary: http://gabriela-gibson.blogspot.com/

Re: logger for Corinthia

Posted by Peter Kelly <pm...@apache.org>.
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
pmkelly@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 <ga...@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/