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@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.