On Thu, Jun 07, 2012 at 01:08:46PM +0200, Martin Minarik wrote:
> This is logging functionality for weston compositor.
> 
> It handles:
> messages coming from libwayland-server from wl_log()
> messages from weston itself, from weston_log()
> 
> Introduce --log option, to specify log file path on the command line.
> When the path is incorrect, or on weston_log_file_destroy(), fall
> back to stderr.
> 
> ---
> 
> In this series there are human readable timestamps, removed
> weston_log_file_create_from_opt() because hardcoded path
> fallback is not needed, append, renamed methods to .._open()
> and .._close(), merge claim_libwayland...() with .._open()

This one looks good now, committed.  We don't need the full date on
each log message, but I think miliseconds would make sense.  I think
we can just print the date and a bit of info (current date, sha1 of
weston repo perhaps), in the beginning of the log file.  Can we do
that?  Also I had to fix a crash, comment below.

Kristian

> ---
>  src/Makefile.am  |    2 +
>  src/compositor.c |    7 +++
>  src/log.c        |  115 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/log.h        |   32 +++++++++++++++
>  4 files changed, 156 insertions(+), 0 deletions(-)
>  create mode 100644 src/log.c
>  create mode 100644 src/log.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 51f7ad5..f3dcb75 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -11,6 +11,8 @@ weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
>  weston_LDADD = $(COMPOSITOR_LIBS) $(DLOPEN_LIBS) -lm ../shared/libshared.la
>  
>  weston_SOURCES =                             \
> +     log.c                                   \
> +     log.h                                   \
>       compositor.c                            \
>       compositor.h                            \
>       filter.c                                \
> diff --git a/src/compositor.c b/src/compositor.c
> index 3039c3d..7d3bad8 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -48,6 +48,7 @@
>  #include <wayland-server.h>
>  #include "compositor.h"
>  #include "../shared/os-compatibility.h"
> +#include "log.h"
>  
>  static struct wl_list child_process_list;
>  static jmp_buf segv_jmp_buf;
> @@ -3110,6 +3111,7 @@ int main(int argc, char *argv[])
>       char *backend = NULL;
>       char *shell = NULL;
>       char *module = NULL;
> +     char *log = NULL;
>       int32_t idle_time = 300;
>       int32_t xserver = 0;
>       char *socket_name = NULL;
> @@ -3130,6 +3132,7 @@ int main(int argc, char *argv[])
>               { WESTON_OPTION_INTEGER, "idle-time", 'i', &idle_time },
>               { WESTON_OPTION_BOOLEAN, "xserver", 0, &xserver },
>               { WESTON_OPTION_STRING, "module", 0, &module },
> +             { WESTON_OPTION_STRING, "log", 0, &log },
>       };
>  
>       argc = parse_options(core_options,
> @@ -3140,6 +3143,8 @@ int main(int argc, char *argv[])
>               exit(EXIT_FAILURE);
>       }
>  
> +     weston_log_file_open(log);
> +
>       display = wl_display_create();
>  
>       loop = wl_display_get_event_loop(display);
> @@ -3240,5 +3245,7 @@ int main(int argc, char *argv[])
>       ec->destroy(ec);
>       wl_display_destroy(display);
>  
> +     weston_log_file_close();
> +
>       return 0;
>  }
> diff --git a/src/log.c b/src/log.c
> new file mode 100644
> index 0000000..0c605ce
> --- /dev/null
> +++ b/src/log.c
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright ?? 2012 Martin Minarik
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and
> + * its documentation for any purpose is hereby granted without fee, provided
> + * that the above copyright notice appear in all copies and that both that
> + * copyright notice and this permission notice appear in supporting
> + * documentation, and that the name of the copyright holders not be used in
> + * advertising or publicity pertaining to distribution of the software
> + * without specific, written prior permission.  The copyright holders make
> + * no representations about the suitability of this software for any
> + * purpose.  It is provided "as is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>
> +
> +#include <wayland-server.h>
> +#include <wayland-util.h>
> +
> +#include "log.h"
> +
> +static FILE *weston_logfile = NULL;
> +
> +static char cached_timestamp[21];
> +static int cached_tv_sec = 0;
> +
> +static int weston_log_timestamp(void)
> +{
> +     struct timespec tp;
> +     unsigned int time;
> +
> +     clock_gettime(CLOCK_REALTIME, &tp);
> +     time = (tp.tv_sec * 1000000L) + (tp.tv_nsec / 1000);
> +
> +     if (cached_tv_sec != tp.tv_sec) {
> +             char string[26];
> +             struct tm *lt = localtime(&tp.tv_sec);
> +
> +             cached_tv_sec = tp.tv_sec;
> +
> +             strftime (string,24,"%Y-%m-%d %H:%M:%S", lt);
> +             strncpy (cached_timestamp, string, 20);
> +     }
> +
> +     return fprintf(weston_logfile, "[%s.%03u] ", cached_timestamp, 
> tp.tv_nsec/1000000);
> +}
> +
> +static int weston_log_print(const char *fmt, va_list arg)
> +{
> +     int l;
> +     l = vfprintf(weston_logfile, fmt, arg);
> +     fflush(weston_logfile);
> +     return l;
> +}
> +
> +static void
> +custom_handler(const char *fmt, va_list arg)
> +{
> +     weston_log_timestamp();
> +     weston_log_print("libwayland: ", NULL);

This crashes on my system.  I don't know that you can just pass NULL
for the va_list here, and it certainly crashes for me.  I just changed
this to calling fprintf(weston_logfile...) directly.

> +     weston_log_print(fmt, arg);
> +}
> +
> +void
> +weston_log_file_open(const char *filename)
> +{
> +     wl_log_set_handler_server(custom_handler);
> +
> +     weston_logfile = fopen(filename, "a");
> +     if (weston_logfile == NULL)
> +             weston_logfile = stderr;
> +}
> +
> +void
> +weston_log_file_close()
> +{
> +     if (weston_logfile != stderr)
> +             fclose(weston_logfile);
> +     weston_logfile = stderr;
> +}
> +
> +WL_EXPORT int
> +weston_log(const char *fmt, ...)
> +{
> +     int l;
> +     va_list argp;
> +     va_start(argp, fmt);
> +     l = weston_log_timestamp();
> +     l += weston_log_print(fmt, argp);
> +     va_end(argp);
> +     return l;
> +}
> +
> +WL_EXPORT int
> +weston_log_continue(const char *fmt, ...)
> +{
> +     int l;
> +     va_list argp;
> +     va_start(argp, fmt);
> +     l = weston_log_print(fmt, argp);
> +     va_end(argp);
> +     return l;
> +}
> diff --git a/src/log.h b/src/log.h
> new file mode 100644
> index 0000000..d050f02
> --- /dev/null
> +++ b/src/log.h
> @@ -0,0 +1,32 @@
> +/*
> + * Copyright ?? 2012 Martin Minarik
> + *
> + * Permission to use, copy, modify, distribute, and sell this software and
> + * its documentation for any purpose is hereby granted without fee, provided
> + * that the above copyright notice appear in all copies and that both that
> + * copyright notice and this permission notice appear in supporting
> + * documentation, and that the name of the copyright holders not be used in
> + * advertising or publicity pertaining to distribution of the software
> + * without specific, written prior permission.  The copyright holders make
> + * no representations about the suitability of this software for any
> + * purpose.  It is provided "as is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#ifndef _WAYLAND_SYSTEM_LOG_H_
> +#define _WAYLAND_SYSTEM_LOG_H_
> +
> +void weston_log_file_open(const char *filename);
> +void weston_log_file_close(void);
> +
> +int weston_log(const char *fmt, ...);
> +int weston_log_continue(const char *fmt, ...);
> +
> +#endif
> -- 
> 1.7.5.4
> 

> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to