[PATCH] wayland: use wl_log instead of printf
Chang Liu
cl91tp at gmail.com
Sun Sep 22 19:22:08 PDT 2013
Anyone has any further comment on this?
On Sep 13, 2013 10:47 PM, "Jason Ekstrand" <jason at jlekstrand.net> 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" <cl91tp at gmail.com> 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
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130923/386e50f7/attachment-0001.html>
More information about the wayland-devel
mailing list