On 08/15/2016 11:21 AM, Lukas Slebodnik wrote:
>
> +void sbus_request_reply_error(struct sbus_request *sbus_req,
> + const char *error_name,
> + const char *fmt,
> + ...);
> +
It would be good to check format string at compile time
; add SSS_ATTRIBUTE_PRINTF
Done.
> DBusError *sbus_error_new(TALLOC_CTX *mem_ctx,
> - const char *dbus_err_name,
> + const char *dbus_error_name,
> const char *fmt,
> ...)
It would be good to change the name also in header file.
Done.
> + sbus_request_fail_and_finish(sbus_req, error);
Is there a special reason why return code of this function is ignored?
If yes then the line deserve a comment.
Because the return code is useless and the function should be void. Feel
free to file a ticket/patch.
>From bca6975c580cbc16957dbb72691dbd16e1b66e7b Mon Sep 17 00:00:00
2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
> Date: Thu, 14 Jul 2016 10:49:37 +0200
> Subject: [PATCH 7/7] sifp: fix coverity warning
>
> sssd-1.14.1/src/lib/sifp/sss_sifp_dbus.c:51: check_return: Calling
"dbus_message_append_args_valist" without checking return value (as is done
elsewhere 4 out of 5 times).
> ---
> src/lib/sifp/sss_sifp_dbus.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/lib/sifp/sss_sifp_dbus.c b/src/lib/sifp/sss_sifp_dbus.c
> index
7c72c52f0d226ccdfaf7b8ffaed7776647a7771c..2906c5ac383c412231127f6ffa8081d47eb2bced 100644
> --- a/src/lib/sifp/sss_sifp_dbus.c
> +++ b/src/lib/sifp/sss_sifp_dbus.c
> @@ -36,6 +36,7 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
> {
> DBusMessage *msg = NULL;
> sss_sifp_error ret;
> + dbus_bool_t bret;
>
> if (object_path == NULL || interface == NULL || method == NULL) {
> return SSS_SIFP_INVALID_ARGUMENT;
> @@ -48,7 +49,11 @@ static sss_sifp_error sss_sifp_ifp_call(sss_sifp_ctx *ctx,
> }
>
> if (first_arg_type != DBUS_TYPE_INVALID) {
> - dbus_message_append_args_valist(msg, first_arg_type, ap);
> + bret = dbus_message_append_args_valist(msg, first_arg_type, ap);
> + if (!bret) {
> + ret = SSS_SIFP_IO_ERROR;
> + goto done;
> + }
> }
>
Is this coverity warning also in master?
If answer is no then It would be better to squash it to the patch which
introduced the warning.
Very likely
sss_sifp: make it compatible with latest version of the infopipe
I don't know, feel free to squash it.