Hi Martin, comments inline.
On Tue, 29 May 2012 21:44:53 +0200 Martin Minarik <[email protected]> wrote: > Changes from last time, we have function > weston_log_file_create_in_home() for opening > log file in home directory, it can handle long file names. > > We have function weston_log_handle_server() to redirect messages from > libwayland-server into log.c:weston_logfile_handler, from > there everything can be redirected as necessary. > --- > src/Makefile.am | 2 + > src/log.c | 97 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/log.h | 33 +++++++++++++++++++ > 3 files changed, 132 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 52457ac..27bac08 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/log.c b/src/log.c > new file mode 100644 > index 0000000..83267d8 > --- /dev/null > +++ b/src/log.c > @@ -0,0 +1,97 @@ > +/* > + * 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 <wayland-server.h> > +#include <wayland-util.h> > + > +#include "log.h" > + > +static FILE *weston_logfile = NULL; > + > +static int > +weston_log_noop_handler(const char *fmt, va_list arg) > +{ > + return 0; > +} Noop handler is not needed in weston, the default should be printing to stderr, unless a log file is specified with a command line option. > + > +static int > +weston_log_fprintf_logfile_handler(const char *fmt, va_list arg) > +{ > + int l; > + l = vfprintf(weston_logfile, fmt, arg); > + fflush(weston_logfile); > + return l; > +} > + > +static wl_log_func_t weston_logfile_handler = weston_log_noop_handler; > + > +void > +weston_log_handle_server() > +{ > + wl_log_set_handler_server(weston_logfile_handler); > +} > + > +void > +weston_log_file_create_in_home(const char *filename) > +{ > + char * home = getenv("HOME"); If HOME is not in the environment, you get NULL, not a pointer to a "". > + if (home[0]) { > + char * string; > + int length = strlen(home) + strlen(filename) + 1; > + string = malloc((length+1) * sizeof(string)); > + snprintf(string, length, "%s%s", home, filename); > + weston_log_file_create(string); > + free(string); > + } > +} I'd really prefer to just get the full file name through a command line option to weston. > + > +void > +weston_log_file_create(const char *filename) > +{ > + weston_logfile = fopen(filename, "w"); Append mode, please, until we have file rotation. > + if (weston_logfile == NULL) > + weston_logfile = stderr; > + weston_logfile_handler = weston_log_fprintf_logfile_handler; > +} > + > +void > +weston_log_file_destroy() > +{ > + weston_logfile_handler = weston_log_noop_handler; > + fclose(weston_logfile); > +} > + > +WL_EXPORT int > +weston_log(const char *fmt, ...) > +{ > + int l; > + va_list argp; > + va_start(argp, fmt); > + l = weston_logfile_handler(fmt, argp); > + va_end(argp); > + return l; > +} > diff --git a/src/log.h b/src/log.h > new file mode 100644 > index 0000000..1b056e8 > --- /dev/null > +++ b/src/log.h > @@ -0,0 +1,33 @@ > +/* > + * 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_handle_server(void); > +void weston_log_file_create_in_home(const char *filename); > +void weston_log_file_create(const char *filename); > +void weston_log_file_destroy(void); > + > +int weston_log(const char *fmt, ...); > + > +#endif You should squash the patch 2/3 into this one, so this code gets used the same time it is added. Patch 3/3 looks good, and due to its size (big) and nature (lots of trivial changes), can stay on its own. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
