Anyone has any further comment on this? On Sep 13, 2013 10:47 PM, "Jason Ekstrand" <[email protected]> wrote:
> I think this is probably pretty good. most of those just look like the > programmer being lazy and not remembering wl_log exists. We may want to > keep it as printf in the cases where it immediately aborts or calls > assert(0). That way it guarantees a message gets printed in the case where > it causes the program to die. > > That said, we may want to make another log function called wl_error or > wl_fatal for actual fatal errors that internally does an assert(0). If we > did add such a function it might want to have it's own log handler. > > Kristian, > Do you have any thoughts? > > Thanks, > --Jason Ekstrand > On Sep 12, 2013 9:36 PM, "Chang Liu" <[email protected]> wrote: > >> use wl_log instead of printf and fprintf in core library >> --- >> I'm pretty sure these printf usages should be avoided since libraries >> should >> not print to stdout. But I'm not sure if there is any reason for favoring >> a >> fprintf to stderr over wl_log. Please enlighten me if there is any. >> >> src/connection.c | 36 ++++++++++++++++++------------------ >> src/event-loop.c | 8 ++++---- >> src/wayland-client.c | 14 ++++++-------- >> 3 files changed, 28 insertions(+), 30 deletions(-) >> >> diff --git a/src/connection.c b/src/connection.c >> index 451b93e..40b7fbd 100644 >> --- a/src/connection.c >> +++ b/src/connection.c >> @@ -512,7 +512,7 @@ wl_closure_marshal(struct wl_object *sender, uint32_t >> opcode, >> >> count = arg_count_for_signature(message->signature); >> if (count > WL_CLOSURE_MAX_ARGS) { >> - printf("too many args (%d)\n", count); >> + wl_log("too many args (%d)\n", count); >> errno = EINVAL; >> return NULL; >> } >> @@ -557,14 +557,14 @@ wl_closure_marshal(struct wl_object *sender, >> uint32_t opcode, >> fd = args[i].h; >> dup_fd = wl_os_dupfd_cloexec(fd, 0); >> if (dup_fd < 0) { >> - fprintf(stderr, "dup failed: %m"); >> + wl_log("dup failed: %m"); >> abort(); >> } >> closure->args[i].h = dup_fd; >> break; >> default: >> - fprintf(stderr, "unhandled format code: '%c'\n", >> - arg.type); >> + wl_log("unhandled format code: '%c'\n", >> + arg.type); >> assert(0); >> break; >> } >> @@ -615,7 +615,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> >> count = arg_count_for_signature(message->signature); >> if (count > WL_CLOSURE_MAX_ARGS) { >> - printf("too many args (%d)\n", count); >> + wl_log("too many args (%d)\n", count); >> errno = EINVAL; >> wl_connection_consume(connection, size); >> return NULL; >> @@ -642,7 +642,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> signature = get_next_argument(signature, &arg); >> >> if (arg.type != 'h' && p + 1 > end) { >> - printf("message too short, " >> + wl_log("message too short, " >> "object (%d), message %s(%s)\n", >> *p, message->name, message->signature); >> errno = EINVAL; >> @@ -669,7 +669,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> >> next = p + DIV_ROUNDUP(length, sizeof *p); >> if (next > end) { >> - printf("message too short, " >> + wl_log("message too short, " >> "object (%d), message %s(%s)\n", >> closure->sender_id, message->name, >> message->signature); >> @@ -680,7 +680,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> s = (char *) p; >> >> if (length > 0 && s[length - 1] != '\0') { >> - printf("string not nul-terminated, " >> + wl_log("string not nul-terminated, " >> "message %s(%s)\n", >> message->name, message->signature); >> errno = EINVAL; >> @@ -695,7 +695,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> closure->args[i].n = id; >> >> if (id == 0 && !arg.nullable) { >> - printf("NULL object received on >> non-nullable " >> + wl_log("NULL object received on >> non-nullable " >> "type, message %s(%s)\n", >> message->name, >> message->signature); >> errno = EINVAL; >> @@ -707,7 +707,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> closure->args[i].n = id; >> >> if (id == 0 && !arg.nullable) { >> - printf("NULL new ID received on >> non-nullable " >> + wl_log("NULL new ID received on >> non-nullable " >> "type, message %s(%s)\n", >> message->name, >> message->signature); >> errno = EINVAL; >> @@ -715,7 +715,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> } >> >> if (wl_map_reserve_new(objects, id) < 0) { >> - printf("not a valid new object id (%d), " >> + wl_log("not a valid new object id (%d), " >> "message %s(%s)\n", >> id, message->name, >> message->signature); >> errno = EINVAL; >> @@ -728,7 +728,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> >> next = p + DIV_ROUNDUP(length, sizeof *p); >> if (next > end) { >> - printf("message too short, " >> + wl_log("message too short, " >> "object (%d), message %s(%s)\n", >> closure->sender_id, message->name, >> message->signature); >> @@ -749,7 +749,7 @@ wl_connection_demarshal(struct wl_connection >> *connection, >> closure->args[i].h = fd; >> break; >> default: >> - printf("unknown type\n"); >> + wl_log("unknown type\n"); >> assert(0); >> break; >> } >> @@ -808,7 +808,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, >> struct wl_map *objects) >> * destroyed client side */ >> object = NULL; >> } else if (object == NULL && id != 0) { >> - printf("unknown object (%u), message >> %s(%s)\n", >> + wl_log("unknown object (%u), message >> %s(%s)\n", >> id, message->name, >> message->signature); >> object = NULL; >> errno = EINVAL; >> @@ -818,7 +818,7 @@ wl_closure_lookup_objects(struct wl_closure *closure, >> struct wl_map *objects) >> if (object != NULL && message->types[i] != NULL && >> !wl_interface_equal((object)->interface, >> message->types[i])) { >> - printf("invalid object (%u), type (%s), " >> + wl_log("invalid object (%u), type (%s), " >> "message %s(%s)\n", >> id, (object)->interface->name, >> message->name, message->signature); >> @@ -884,7 +884,7 @@ convert_arguments_to_ffi(const char *signature, >> uint32_t flags, >> ffi_args[i] = &args[i].h; >> break; >> default: >> - printf("unknown type\n"); >> + wl_log("unknown type\n"); >> assert(0); >> break; >> } >> @@ -944,8 +944,8 @@ copy_fds_to_connection(struct wl_closure *closure, >> >> fd = closure->args[i].h; >> if (wl_connection_put_fd(connection, fd)) { >> - fprintf(stderr, "request could not be marshaled: " >> - "can't send file descriptor"); >> + wl_log("request could not be marshaled: " >> + "can't send file descriptor"); >> return -1; >> } >> } >> diff --git a/src/event-loop.c b/src/event-loop.c >> index d323601..43d4f50 100644 >> --- a/src/event-loop.c >> +++ b/src/event-loop.c >> @@ -97,7 +97,7 @@ add_source(struct wl_event_loop *loop, >> struct epoll_event ep; >> >> if (source->fd < 0) { >> - fprintf(stderr, "could not add source\n: %m"); >> + wl_log("could not add source\n: %m"); >> free(source); >> return NULL; >> } >> @@ -175,7 +175,7 @@ wl_event_source_timer_dispatch(struct wl_event_source >> *source, >> len = read(source->fd, &expires, sizeof expires); >> if (len != sizeof expires) >> /* Is there anything we can do here? Will this ever >> happen? */ >> - fprintf(stderr, "timerfd read error: %m\n"); >> + wl_log("timerfd read error: %m\n"); >> >> return timer_source->func(timer_source->base.data); >> } >> @@ -212,7 +212,7 @@ wl_event_source_timer_update(struct wl_event_source >> *source, int ms_delay) >> its.it_value.tv_sec = ms_delay / 1000; >> its.it_value.tv_nsec = (ms_delay % 1000) * 1000 * 1000; >> if (timerfd_settime(source->fd, 0, &its, NULL) < 0) { >> - fprintf(stderr, "could not set timerfd\n: %m"); >> + wl_log("could not set timerfd\n: %m"); >> return -1; >> } >> >> @@ -237,7 +237,7 @@ wl_event_source_signal_dispatch(struct >> wl_event_source *source, >> len = read(source->fd, &signal_info, sizeof signal_info); >> if (len != sizeof signal_info) >> /* Is there anything we can do here? Will this ever >> happen? */ >> - fprintf(stderr, "signalfd read error: %m\n"); >> + wl_log("signalfd read error: %m\n"); >> >> return signal_source->func(signal_source->signal_number, >> signal_source->base.data); >> diff --git a/src/wayland-client.c b/src/wayland-client.c >> index 04d988b..9633862 100644 >> --- a/src/wayland-client.c >> +++ b/src/wayland-client.c >> @@ -318,7 +318,7 @@ wl_proxy_add_listener(struct wl_proxy *proxy, >> void (**implementation)(void), void *data) >> { >> if (proxy->object.implementation || proxy->dispatcher) { >> - fprintf(stderr, "proxy already has listener\n"); >> + wl_log("proxy already has listener\n"); >> return -1; >> } >> >> @@ -371,7 +371,7 @@ wl_proxy_add_dispatcher(struct wl_proxy *proxy, >> const void *implementation, void *data) >> { >> if (proxy->object.implementation || proxy->dispatcher) { >> - fprintf(stderr, "proxy already has listener\n"); >> + wl_log("proxy already has listener\n"); >> return -1; >> } >> >> @@ -453,7 +453,7 @@ wl_proxy_marshal_array(struct wl_proxy *proxy, >> uint32_t opcode, >> >> &proxy->object.interface->methods[opcode]); >> >> if (closure == NULL) { >> - fprintf(stderr, "Error marshalling request\n"); >> + wl_log("error marshalling request\n"); >> abort(); >> } >> >> @@ -461,7 +461,7 @@ wl_proxy_marshal_array(struct wl_proxy *proxy, >> uint32_t opcode, >> wl_closure_print(closure, &proxy->object, true); >> >> if (wl_closure_send(closure, proxy->display->connection)) { >> - fprintf(stderr, "Error sending request: %m\n"); >> + wl_log("error sending request: %m\n"); >> abort(); >> } >> >> @@ -532,8 +532,7 @@ connect_to_socket(const char *name) >> >> runtime_dir = getenv("XDG_RUNTIME_DIR"); >> if (!runtime_dir) { >> - fprintf(stderr, >> - "error: XDG_RUNTIME_DIR not set in the >> environment.\n"); >> + wl_log("error: XDG_RUNTIME_DIR not set in the >> environment.\n"); >> >> /* to prevent programs reporting >> * "failed to create display: Success" */ >> @@ -558,8 +557,7 @@ connect_to_socket(const char *name) >> >> assert(name_size > 0); >> if (name_size > (int)sizeof addr.sun_path) { >> - fprintf(stderr, >> - "error: socket path \"%s/%s\" plus null terminator" >> + wl_log("error: socket path \"%s/%s\" plus null terminator" >> " exceeds 108 bytes\n", runtime_dir, name); >> close(fd); >> /* to prevent programs reporting >> -- >> 1.8.3.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
