libseat: Improve logging with seatd conn helpers

Add helpers around connection access to have all logging centralized and
reduce code duplication. Improve existing helpers to further reduce code
duplication.

The seatd backend should have much better logging after this.
This commit is contained in:
Kenny Levinsen 2020-08-29 20:31:51 +02:00
parent 69d57aaf33
commit 8b4d139873
3 changed files with 102 additions and 106 deletions

View file

@ -284,15 +284,14 @@ int connection_get(struct connection *connection, void *dst, size_t count) {
return count; return count;
} }
int connection_get_fd(struct connection *connection) { int connection_get_fd(struct connection *connection, int *fd) {
int fd; if (sizeof(int) > connection_buffer_size(&connection->fds_in)) {
if (sizeof fd > connection_buffer_size(&connection->fds_in)) {
errno = EAGAIN; errno = EAGAIN;
return -1; return -1;
} }
connection_buffer_copy(&connection->fds_in, &fd, sizeof fd); connection_buffer_copy(&connection->fds_in, fd, sizeof(int));
connection_buffer_consume(&connection->fds_in, sizeof fd); connection_buffer_consume(&connection->fds_in, sizeof(int));
return fd; return 0;
} }
void connection_close_fds(struct connection *connection) { void connection_close_fds(struct connection *connection) {

View file

@ -28,7 +28,7 @@ int connection_put_fd(struct connection *connection, int fd);
size_t connection_pending(struct connection *connection); size_t connection_pending(struct connection *connection);
int connection_get(struct connection *connection, void *dst, size_t count); int connection_get(struct connection *connection, void *dst, size_t count);
int connection_get_fd(struct connection *connection); int connection_get_fd(struct connection *connection, int *fd);
void connection_restore(struct connection *connection, size_t count); void connection_restore(struct connection *connection, size_t count);
void connection_close_fds(struct connection *connection); void connection_close_fds(struct connection *connection);

View file

@ -58,9 +58,11 @@ static int seatd_connect(void) {
} addr = {{0}}; } addr = {{0}};
int fd = socket(AF_UNIX, SOCK_STREAM, 0); int fd = socket(AF_UNIX, SOCK_STREAM, 0);
if (fd == -1) { if (fd == -1) {
log_errorf("Could not create socket: %s", strerror(errno));
return -1; return -1;
} }
if (set_nonblock(fd) == -1) { if (set_nonblock(fd) == -1) {
log_errorf("Could not make socket non-blocking: %s", strerror(errno));
close(fd); close(fd);
return -1; return -1;
} }
@ -72,6 +74,7 @@ static int seatd_connect(void) {
strncpy(addr.unix.sun_path, path, sizeof addr.unix.sun_path); strncpy(addr.unix.sun_path, path, sizeof addr.unix.sun_path);
socklen_t size = offsetof(struct sockaddr_un, sun_path) + strlen(addr.unix.sun_path); socklen_t size = offsetof(struct sockaddr_un, sun_path) + strlen(addr.unix.sun_path);
if (connect(fd, &addr.generic, size) == -1) { if (connect(fd, &addr.generic, size) == -1) {
log_debugf("Could not connect to socket: %s", strerror(errno));
close(fd); close(fd);
return -1; return -1;
}; };
@ -88,21 +91,54 @@ static struct backend_seatd *backend_seatd_from_libseat_backend(struct libseat *
return (struct backend_seatd *)base; return (struct backend_seatd *)base;
} }
static size_t read_header(struct connection *connection, uint16_t expected_opcode) { static inline int conn_put(struct backend_seatd *backend, const void *data, const size_t data_len) {
if (connection_put(&backend->connection, data, data_len) == -1) {
log_errorf("Could not send request: %s", strerror(errno));
return -1;
}
return 0;
}
static inline int conn_flush(struct backend_seatd *backend) {
if (connection_flush(&backend->connection) == -1) {
log_errorf("Could not flush connection: %s", strerror(errno));
return -1;
}
return 0;
}
static inline int conn_get(struct backend_seatd *backend, void *target, const size_t target_len) {
if (connection_get(&backend->connection, target, target_len) == -1) {
log_error("Invalid message: insufficient data received");
errno = EBADMSG;
return -1;
}
return 0;
}
static inline int conn_get_fd(struct backend_seatd *backend, int *fd) {
if (connection_get_fd(&backend->connection, fd) == -1) {
log_error("Invalid message: insufficient data received");
errno = EBADMSG;
return -1;
}
return 0;
}
static size_t read_header(struct backend_seatd *backend, uint16_t expected_opcode,
size_t expected_size, bool variable) {
struct proto_header header; struct proto_header header;
if (connection_get(connection, &header, sizeof header) == -1) { if (conn_get(backend, &header, sizeof header) == -1) {
log_error("Received invalid message: header too short");
return SIZE_MAX; return SIZE_MAX;
} }
if (header.opcode != expected_opcode) { if (header.opcode != expected_opcode) {
connection_restore(connection, sizeof header); connection_restore(&backend->connection, sizeof header);
struct proto_server_error msg; struct proto_server_error msg;
if (header.opcode != SERVER_ERROR) { if (header.opcode != SERVER_ERROR) {
log_errorf("Received invalid message: expected opcode %d, received opcode %d", log_errorf("Unexpected response: expected opcode %d, received opcode %d",
expected_opcode, header.opcode); expected_opcode, header.opcode);
errno = EBADMSG; errno = EBADMSG;
} else if (connection_get(connection, &msg, sizeof msg) == -1) { } else if (conn_get(backend, &msg, sizeof msg) == -1) {
log_error("Received invalid message");
errno = EBADMSG; errno = EBADMSG;
} else { } else {
errno = msg.error_code; errno = msg.error_code;
@ -110,12 +146,19 @@ static size_t read_header(struct connection *connection, uint16_t expected_opcod
return SIZE_MAX; return SIZE_MAX;
} }
if ((!variable && header.size != expected_size) || (variable && header.size < expected_size)) {
log_errorf("Invalid message: does not match expected size: variable: %d, header.size: %d, expected size: %zd",
variable, header.size, expected_size);
errno = EBADMSG;
return SIZE_MAX;
}
return header.size; return header.size;
} }
static int queue_event(struct backend_seatd *backend, int opcode) { static int queue_event(struct backend_seatd *backend, int opcode) {
struct pending_event *ev = calloc(1, sizeof(struct pending_event)); struct pending_event *ev = calloc(1, sizeof(struct pending_event));
if (ev == NULL) { if (ev == NULL) {
log_errorf("Allocation failed: %s", strerror(errno));
return -1; return -1;
} }
@ -211,8 +254,7 @@ static int poll_connection(struct backend_seatd *backend, int timeout) {
} }
static int dispatch(struct backend_seatd *backend) { static int dispatch(struct backend_seatd *backend) {
if (connection_flush(&backend->connection) == -1) { if (conn_flush(backend) == -1) {
log_errorf("Could not flush connection: %s", strerror(errno));
return -1; return -1;
} }
int opcode = 0, res = 0; int opcode = 0, res = 0;
@ -278,9 +320,10 @@ static struct libseat *_open_seat(struct libseat_seat_listener *listener, void *
assert(listener->enable_seat != NULL && listener->disable_seat != NULL); assert(listener->enable_seat != NULL && listener->disable_seat != NULL);
struct backend_seatd *backend = calloc(1, sizeof(struct backend_seatd)); struct backend_seatd *backend = calloc(1, sizeof(struct backend_seatd));
if (backend == NULL) { if (backend == NULL) {
close(fd); log_errorf("Allocation failed: %s", strerror(errno));
return NULL; goto alloc_error;
} }
backend->seat_listener = listener; backend->seat_listener = listener;
backend->seat_listener_data = data; backend->seat_listener_data = data;
backend->connection.fd = fd; backend->connection.fd = fd;
@ -291,42 +334,31 @@ static struct libseat *_open_seat(struct libseat_seat_listener *listener, void *
.opcode = CLIENT_OPEN_SEAT, .opcode = CLIENT_OPEN_SEAT,
.size = 0, .size = 0,
}; };
if (conn_put(backend, &header, sizeof header) == -1 || dispatch(backend) == -1) {
if (connection_put(&backend->connection, &header, sizeof header) == -1 || goto backend_error;
dispatch(backend) == -1) {
destroy(backend);
return NULL;
}
size_t size = read_header(&backend->connection, SERVER_SEAT_OPENED);
if (size == SIZE_MAX) {
destroy(backend);
return NULL;
} }
struct proto_server_seat_opened rmsg; struct proto_server_seat_opened rmsg;
if (sizeof rmsg > size) { size_t size = read_header(backend, SERVER_SEAT_OPENED, sizeof rmsg, true);
goto badmsg_error; if (size == SIZE_MAX || conn_get(backend, &rmsg, sizeof rmsg) == -1) {
goto backend_error;
} }
if (rmsg.seat_name_len != size - sizeof rmsg) {
if (connection_get(&backend->connection, &rmsg, sizeof rmsg) == -1) { log_errorf("Invalid message: seat_name_len does not match remaining message size (%d != %zd)",
goto badmsg_error; rmsg.seat_name_len, size);
}; errno = EBADMSG;
goto backend_error;
if (sizeof rmsg + rmsg.seat_name_len > size || }
rmsg.seat_name_len >= sizeof backend->seat_name) { if (conn_get(backend, backend->seat_name, rmsg.seat_name_len) == -1) {
goto badmsg_error; goto backend_error;
} }
if (connection_get(&backend->connection, backend->seat_name, rmsg.seat_name_len) == -1) {
goto badmsg_error;
};
return &backend->base; return &backend->base;
badmsg_error: backend_error:
log_error("Received invalid message"); destroy(backend);
errno = EBADMSG; alloc_error:
close(fd);
return NULL; return NULL;
} }
@ -346,21 +378,20 @@ static int close_seat(struct libseat *base) {
.opcode = CLIENT_CLOSE_SEAT, .opcode = CLIENT_CLOSE_SEAT,
.size = 0, .size = 0,
}; };
if (conn_put(backend, &header, sizeof header) == -1 || dispatch(backend) == -1) {
if (connection_put(&backend->connection, &header, sizeof header) == -1 || goto error;
dispatch(backend) == -1) {
destroy(backend);
return -1;
} }
size_t size = read_header(&backend->connection, SERVER_SEAT_CLOSED); if (read_header(backend, SERVER_SEAT_CLOSED, 0, false) == SIZE_MAX) {
if (size == SIZE_MAX) { goto error;
destroy(backend);
return -1;
} }
destroy(backend); destroy(backend);
return 0; return 0;
error:
destroy(backend);
return -1;
} }
static const char *seat_name(struct libseat *base) { static const char *seat_name(struct libseat *base) {
@ -384,38 +415,19 @@ static int open_device(struct libseat *base, const char *path, int *fd) {
.opcode = CLIENT_OPEN_DEVICE, .opcode = CLIENT_OPEN_DEVICE,
.size = sizeof msg + pathlen, .size = sizeof msg + pathlen,
}; };
if (conn_put(backend, &header, sizeof header) == -1 ||
if (connection_put(&backend->connection, &header, sizeof header) == -1 || conn_put(backend, &msg, sizeof msg) == -1 || conn_put(backend, path, pathlen) == -1 ||
connection_put(&backend->connection, &msg, sizeof msg) == -1 || dispatch(backend) == -1) {
connection_put(&backend->connection, path, pathlen) == -1 || dispatch(backend) == -1) {
return -1;
}
size_t size = read_header(&backend->connection, SERVER_DEVICE_OPENED);
if (size == SIZE_MAX) {
return -1; return -1;
} }
struct proto_server_device_opened rmsg; struct proto_server_device_opened rmsg;
if (sizeof rmsg > size) { if (read_header(backend, SERVER_DEVICE_OPENED, sizeof rmsg, false) == SIZE_MAX ||
goto badmsg_error; conn_get(backend, &rmsg, sizeof rmsg) == -1 || conn_get_fd(backend, fd)) {
}
if (connection_get(&backend->connection, &rmsg, sizeof rmsg) == -1) {
goto badmsg_error;
}
int received_fd = connection_get_fd(&backend->connection);
if (received_fd == -1) {
goto badmsg_error;
}
*fd = received_fd;
return rmsg.device_id;
badmsg_error:
log_error("Received invalid message");
errno = EBADMSG;
return -1; return -1;
}
return rmsg.device_id;
} }
static int close_device(struct libseat *base, int device_id) { static int close_device(struct libseat *base, int device_id) {
@ -432,34 +444,23 @@ static int close_device(struct libseat *base, int device_id) {
.opcode = CLIENT_CLOSE_DEVICE, .opcode = CLIENT_CLOSE_DEVICE,
.size = sizeof msg, .size = sizeof msg,
}; };
if (conn_put(backend, &header, sizeof header) == -1 ||
if (connection_put(&backend->connection, &header, sizeof header) == -1 || conn_put(backend, &msg, sizeof msg) == -1 || dispatch(backend) == -1) {
connection_put(&backend->connection, &msg, sizeof msg) == -1 || dispatch(backend) == -1) {
return -1;
}
size_t size = read_header(&backend->connection, SERVER_DEVICE_CLOSED);
if (size == SIZE_MAX) {
return -1; return -1;
} }
struct proto_server_device_closed rmsg; struct proto_server_device_closed rmsg;
if (sizeof rmsg > size) { if (read_header(backend, SERVER_DEVICE_CLOSED, sizeof rmsg, false) == SIZE_MAX ||
goto badmsg_error; conn_get(backend, &rmsg, sizeof rmsg) == -1) {
} return -1;
if (connection_get(&backend->connection, &rmsg, sizeof rmsg) == -1) {
goto badmsg_error;
} }
if (rmsg.device_id != device_id) { if (rmsg.device_id != device_id) {
goto badmsg_error; log_errorf("Unexpected response: expected device close for %d, got device close for %d",
rmsg.device_id, device_id);
return -1;
} }
return 0; return 0;
badmsg_error:
log_error("Received invalid message");
errno = EBADMSG;
return -1;
} }
static int switch_session(struct libseat *base, int session) { static int switch_session(struct libseat *base, int session) {
@ -475,10 +476,8 @@ static int switch_session(struct libseat *base, int session) {
.opcode = CLIENT_SWITCH_SESSION, .opcode = CLIENT_SWITCH_SESSION,
.size = sizeof msg, .size = sizeof msg,
}; };
if (conn_put(backend, &header, sizeof header) == -1 ||
if (connection_put(&backend->connection, &header, sizeof header) == -1 || conn_put(backend, &msg, sizeof msg) == -1 || conn_flush(backend) == -1) {
connection_put(&backend->connection, &msg, sizeof msg) == -1 ||
connection_flush(&backend->connection) == -1) {
return -1; return -1;
} }
@ -491,9 +490,7 @@ static int disable_seat(struct libseat *base) {
.opcode = CLIENT_DISABLE_SEAT, .opcode = CLIENT_DISABLE_SEAT,
.size = 0, .size = 0,
}; };
if (conn_put(backend, &header, sizeof header) == -1 || conn_flush(backend) == -1) {
if (connection_put(&backend->connection, &header, sizeof header) == -1 ||
connection_flush(&backend->connection) == -1) {
return -1; return -1;
} }