Re: [PATCH] wayland: use wl_log instead of printf
Anyone has any further comment on this?
On Sep 13, 2013 10:47 PM, "Jason Ekstrand" 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" 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
>> no
Re: [PATCH] wayland: use wl_log instead of printf
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" 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,
>
[PATCH] wayland: use wl_log instead of printf
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("n
