wayland-rs: Add more eprintln!() messages for protocol problems

In particular, rust-impl wayland-client should eprintln!() the issues when it encounters a problem with what the server has sent.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 16 (16 by maintainers)

Most upvoted comments

#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>
#include <error.h>

typedef void wl_log_func_t(char const *, va_list);

void wl_log_funt(char const *fmt, va_list list);
// All errors are fatal
void wl_log_funt(char const *fmt, va_list list) {
    va_list cpy;
    va_copy(cpy, list);
    int expected_size = vsnprintf(NULL, 0, fmt, cpy);
    if (expected_size < 0) {
        error(EXIT_FAILURE, expected_size, "Unexpected Error");
    }
    va_end(cpy);
    
    size_t buffer_size = (size_t)expected_size + 1;
    char *buffer = malloc(sizeof *buffer * buffer_size);
    if (buffer == NULL) {
        error(EXIT_FAILURE, 0, "Out of memory");
    }
    
    int result_size = vsnprintf(buffer, buffer_size, fmt, list);  
    if (result_size != expected_size) {
        free(buffer);
        if (result_size < 0) {
            error(EXIT_FAILURE, result_size, "Unexpected Error");
        }
        else {
            error(EXIT_FAILURE, 0,
                  "vsnprintf return is not valid, expected %d, result %d",
                  expected_size, result_size);
        }
    }
    // call rust
    printf("%s", buffer);
    free(buffer);
}

static void wrap(wl_log_func_t *f, char const *fmt, ...) {
    va_list list;
    va_start(list, fmt);
    (*f)(fmt, list);
    va_end(list);
}

int main(void)
{
    wrap(wl_log_funt, "%s: %d", "Long time no C", 42);
}

The problem is that is allocate memory but I least it’s work well.

It’s hard to answer these questions:

  1. I don’t think this crate should copy the coding style of C implementation of wayland (for many reason :p), that of course my opinion, I believe your goal was to have a pure Rust implementation, so have better code than C, not to limit it to C feature. That should include logging strategy.
  2. Handle the log config and so WAYLAND_DEBUG must be transfer to the user control (some helper function not a big deal I think) cause user in Rust have the logging control, the library have no control.
  3. I don’t think it’s inconsistency to have a different log output if the user enable a compile time feature of a crate. Also, it’s about logging so not a big deal in my opinion. As I say the logging configuration should be let to user, and so the user can handle it as wanted.

I understand the problems you raise so I think this would need a proof of concept PR to see what it’s could look like.

Use case:

  • Rust call wayland-rs: Can use any log config without problem.
  • Something call wayland-rs: Problem cause Something need to init a logger somehow. Maybe using a feature ?
  • Rust call wayland-rs using system-lib: potentially mixing log format output, could be handle by the user but meh. Already happen in a lot of other solution.

We could also have a feature that initialize the logging system (for foreign code) in the same place where the crate already check the WAYLAND_DEBUG but it’s wrong to do that https://github.com/Smithay/wayland-rs/blob/c080f7519aa9e0ec60051f3342f90289c7c77cda/wayland-client/src/rust_imp/display.rs#L30

We could use a feature that switch between the two logging strategy. That require more code.

We could not care.

I’m indecisive about that. These messages are printed in result of a protocol error, meaning that the connection is going to be closed, and in 99% of cases meaning the client will panic!() because event_queue.dispatch() returns an error.

Also, such messages being triggered is 100% guaranteed to be because of a bug (either server-side or client-side), from my point of view they mostly would serve as additional context to a panic backtrace, so I’m not sure what would be actually gained from pulling in the log machinery, and requiring downstream apps to use it to see these messages.