[PATCH] SBUS: Fir error handling after closing container - sssd-devel - Fedora mailing-lists

List overview All Threads
Download

[PATCH] SBUS: Fir error handling after closing container

[PATCH] TESTS: Add tests for the...

[PATCH] GPO: Terminate request on...

Lukas Slebodnik

11 Oct 2014 11 Oct '14

4:43 p.m.

ehlo,

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

simple patch is attached.

LS

Attachments:

0002-SBUS-Fir-error-handling-after-closing-contaner.patch (text/plain — 1.0 KB)

Reply

Show replies by date

Jakub Hrozek

12 Oct 12 Oct

9:52 a.m.

On Sat, Oct 11, 2014 at 11:43:28PM +0200, Lukas Slebodnik wrote:

ehlo,

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

simple patch is attached.

LS

ACK

Reply

Jakub Hrozek

9:58 a.m.

On Sun, Oct 12, 2014 at 04:52:15PM +0200, Jakub Hrozek wrote:

On Sat, Oct 11, 2014 at 11:43:28PM +0200, Lukas Slebodnik wrote:

...
ehlo,

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

simple patch is attached.

LS

ACK

Sorry, I meant to ack another patch.

For this one, the other reply about adding additional goto is valid.

Reply

Jakub Hrozek

9:54 a.m.

On Sat, Oct 11, 2014 at 11:43:28PM +0200, Lukas Slebodnik wrote:

ehlo,

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

simple patch is attached.

LS

Can you also add goto done to the next EINVAL handler and ret=EOK just before the done: label? That way the code will also be safe against future additions.

Reply

Lukas Slebodnik

13 Oct 13 Oct

2:51 a.m.

On (12/10/14 16:54), Jakub Hrozek wrote:

On Sat, Oct 11, 2014 at 11:43:28PM +0200, Lukas Slebodnik wrote:

...
ehlo,

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

simple patch is attached.

LS

Can you also add goto done to the next EINVAL handler and ret=EOK just before the done: label? That way the code will also be safe against future additions.

Sure,

updated patch is attached.

LS

Reply

Jakub Hrozek

14 Oct 14 Oct

7:40 a.m.

On Mon, Oct 13, 2014 at 09:51:17AM +0200, Lukas Slebodnik wrote:

From fac3031392c09ab1827e28311eaa0addbfb5fc46 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Fri, 10 Oct 2014 19:23:33 +0200 Subject: [PATCH] SBUS: Fir error handling after closing container

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

src/sbus/sssd_dbus_request.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index 7729d4e0d7bf6e517e2efce4dbeb064f6f471b87..677ed532f7555f6aeba378ebd9a0b06167ddfa1b 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -286,6 +286,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close array\n")); ret = EINVAL;
   goto done;
}

dbret = dbus_message_iter_close_container(&iter, &variant_iter);
@@ -298,6 +299,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close variant\n")); ret = EINVAL;
   goto done;
}

Can you also add ret=EOK before the done label?

done:

2.1.0

Reply

Lukas Slebodnik

7:53 a.m.

On (14/10/14 14:40), Jakub Hrozek wrote:

On Mon, Oct 13, 2014 at 09:51:17AM +0200, Lukas Slebodnik wrote:

...
From fac3031392c09ab1827e28311eaa0addbfb5fc46 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Fri, 10 Oct 2014 19:23:33 +0200 Subject: [PATCH] SBUS: Fir error handling after closing container

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

src/sbus/sssd_dbus_request.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index 7729d4e0d7bf6e517e2efce4dbeb064f6f471b87..677ed532f7555f6aeba378ebd9a0b06167ddfa1b 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -286,6 +286,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close array\n")); ret = EINVAL;
   goto done;
}

dbret = dbus_message_iter_close_container(&iter, &variant_iter);
@@ -298,6 +299,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close variant\n")); ret = EINVAL;
   goto done;
}
Can you also add ret=EOK before the done label?

It would override return value from function sbus_request_finish.

292 dbret = dbus_message_iter_close_container(&iter, &variant_iter); 293 if (dbret) { 294 ret = sbus_request_finish(dbus_req, reply); 295 } else { 296 sbus_request_fail_and_finish( 297 dbus_req, 298 sbus_error_new(dbus_req, 299 DBUS_ERROR_FAILED, 300 "Could not close variant\n")); 301 ret = EINVAL; 302 goto done; 303 } 304 305 done: 306 if (reply != NULL) { 307 dbus_message_unref(reply); 308 }

LS

Reply

Jakub Hrozek

22 Oct 22 Oct

9:17 a.m.

On Tue, Oct 14, 2014 at 02:53:48PM +0200, Lukas Slebodnik wrote:

On (14/10/14 14:40), Jakub Hrozek wrote:

...
On Mon, Oct 13, 2014 at 09:51:17AM +0200, Lukas Slebodnik wrote:

...
From fac3031392c09ab1827e28311eaa0addbfb5fc46 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Fri, 10 Oct 2014 19:23:33 +0200 Subject: [PATCH] SBUS: Fir error handling after closing container

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

src/sbus/sssd_dbus_request.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index 7729d4e0d7bf6e517e2efce4dbeb064f6f471b87..677ed532f7555f6aeba378ebd9a0b06167ddfa1b 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -286,6 +286,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close array\n")); ret = EINVAL;
   goto done;
}

dbret = dbus_message_iter_close_container(&iter, &variant_iter);
@@ -298,6 +299,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close variant\n")); ret = EINVAL;
   goto done;
}
Can you also add ret=EOK before the done label?
It would override return value from function sbus_request_finish.

Ah, OK.

ACK, then.

I'll just fix the typo in the commit message (s/Fir/Fix)

Reply

Jakub Hrozek

2:51 p.m.

On Wed, Oct 22, 2014 at 04:17:30PM +0200, Jakub Hrozek wrote:

On Tue, Oct 14, 2014 at 02:53:48PM +0200, Lukas Slebodnik wrote:

...
On (14/10/14 14:40), Jakub Hrozek wrote:

...
On Mon, Oct 13, 2014 at 09:51:17AM +0200, Lukas Slebodnik wrote:

...
From fac3031392c09ab1827e28311eaa0addbfb5fc46 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Fri, 10 Oct 2014 19:23:33 +0200 Subject: [PATCH] SBUS: Fir error handling after closing container

If function dbus_message_iter_close_container fail the return variable ret will be set to EINVAL, but function will not be immediately terminated. "goto done" was missing.

src/sbus/sssd_dbus_request.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/sbus/sssd_dbus_request.c b/src/sbus/sssd_dbus_request.c index 7729d4e0d7bf6e517e2efce4dbeb064f6f471b87..677ed532f7555f6aeba378ebd9a0b06167ddfa1b 100644 --- a/src/sbus/sssd_dbus_request.c +++ b/src/sbus/sssd_dbus_request.c @@ -286,6 +286,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close array\n")); ret = EINVAL;
   goto done;
}

dbret = dbus_message_iter_close_container(&iter, &variant_iter);
@@ -298,6 +299,7 @@ int sbus_request_return_array_as_variant(struct sbus_request *dbus_req, DBUS_ERROR_FAILED, "Could not close variant\n")); ret = EINVAL;
   goto done;
}
Can you also add ret=EOK before the done label?
It would override return value from function sbus_request_finish.
Ah, OK.

ACK, then.

I'll just fix the typo in the commit message (s/Fir/Fix)

* master: 82b5395c1519b9392ddd323ece0845b51a994bbc

Reply

3889

Age (days ago)

3900

Last active (days ago)

sssd-devel@lists.fedorahosted.org

8 comments

2 participants

tags (0)

participants (2)

Jakub Hrozek
Lukas Slebodnik