All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/1] s390x/event-facility.c: remove unneeded labels
@ 2020-01-08 14:46 Daniel Henrique Barboza
  2020-01-13 16:02 ` Thomas Huth
  2020-01-13 16:52 ` Cornelia Huck
  0 siblings, 2 replies; 3+ messages in thread
From: Daniel Henrique Barboza @ 2020-01-08 14:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Halil Pasic, Christian Borntraeger, Thomas Huth,
	Daniel Henrique Barboza, Cornelia Huck

'out' label from write_event_mask() and and write_event_data()
can be replaced by 'return'.

The 'out' label from read_event_data() can also be replaced.
However, as suggested by Cornelia Huck, instead of simply
replacing the 'out' label, let's also change the code flow
a bit to make it clearer that sccb events are always handled
regardless of the mask for unconditional reads, while selective
reads are handled if the mask is valid.

CC: Cornelia Huck <cohuck@redhat.com>
CC: Thomas Huth <thuth@redhat.com>
CC: Halil Pasic <pasic@linux.ibm.com>
CC: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---

This is the v2 of patch 37 sent in the series:

https://patchwork.kernel.org/cover/11319771/

after this review from Cornelia:

https://patchwork.kernel.org/patch/11319847/#23088037

 hw/s390x/event-facility.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 6afe278cad..8a93b8a1da 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -182,11 +182,11 @@ static void write_event_data(SCLPEventFacility *ef, SCCB *sccb)
 {
     if (sccb->h.function_code != SCLP_FC_NORMAL_WRITE) {
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
-        goto out;
+        return;
     }
     if (be16_to_cpu(sccb->h.length) < 8) {
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
-        goto out;
+        return;
     }
     /* first do a sanity check of the write events */
     sccb->h.response_code = cpu_to_be16(write_event_length_check(sccb));
@@ -196,9 +196,6 @@ static void write_event_data(SCLPEventFacility *ef, SCCB *sccb)
         sccb->h.response_code =
                 cpu_to_be16(handle_sccb_write_events(ef, sccb));
     }
-
-out:
-    return;
 }
 
 static uint16_t handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
@@ -262,17 +259,18 @@ static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
 
     if (be16_to_cpu(sccb->h.length) != SCCB_SIZE) {
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
-        goto out;
+        return;
     }
 
-    sclp_cp_receive_mask = ef->receive_mask;
-
-    /* get active selection mask */
     switch (sccb->h.function_code) {
     case SCLP_UNCONDITIONAL_READ:
-        sclp_active_selection_mask = sclp_cp_receive_mask;
+        sccb->h.response_code = cpu_to_be16(
+            handle_sccb_read_events(ef, sccb, ef->receive_mask));
         break;
     case SCLP_SELECTIVE_READ:
+        /* get active selection mask */
+        sclp_cp_receive_mask = ef->receive_mask;
+
         copy_mask((uint8_t *)&sclp_active_selection_mask, (uint8_t *)&red->mask,
                   sizeof(sclp_active_selection_mask), ef->mask_length);
         sclp_active_selection_mask = be64_to_cpu(sclp_active_selection_mask);
@@ -280,18 +278,14 @@ static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
             (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
             sccb->h.response_code =
                     cpu_to_be16(SCLP_RC_INVALID_SELECTION_MASK);
-            goto out;
+        } else {
+            sccb->h.response_code = cpu_to_be16(
+                handle_sccb_read_events(ef, sccb, sclp_active_selection_mask));
         }
         break;
     default:
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
-        goto out;
     }
-    sccb->h.response_code = cpu_to_be16(
-            handle_sccb_read_events(ef, sccb, sclp_active_selection_mask));
-
-out:
-    return;
 }
 
 static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
@@ -303,7 +297,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
     if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
         ((mask_length != 4) && !ef->allow_all_mask_sizes)) {
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
-        goto out;
+        return;
     }
 
     /*
@@ -328,9 +322,6 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
 
     sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
     ef->mask_length = mask_length;
-
-out:
-    return;
 }
 
 /* qemu object creation and initialization functions */
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] s390x/event-facility.c: remove unneeded labels
  2020-01-08 14:46 [PATCH v2 1/1] s390x/event-facility.c: remove unneeded labels Daniel Henrique Barboza
@ 2020-01-13 16:02 ` Thomas Huth
  2020-01-13 16:52 ` Cornelia Huck
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Huth @ 2020-01-13 16:02 UTC (permalink / raw)
  To: Daniel Henrique Barboza, qemu-devel
  Cc: Halil Pasic, Christian Borntraeger, Cornelia Huck

On 08/01/2020 15.46, Daniel Henrique Barboza wrote:
> 'out' label from write_event_mask() and and write_event_data()
> can be replaced by 'return'.
> 
> The 'out' label from read_event_data() can also be replaced.
> However, as suggested by Cornelia Huck, instead of simply
> replacing the 'out' label, let's also change the code flow
> a bit to make it clearer that sccb events are always handled
> regardless of the mask for unconditional reads, while selective
> reads are handled if the mask is valid.
> 
> CC: Cornelia Huck <cohuck@redhat.com>
> CC: Thomas Huth <thuth@redhat.com>
> CC: Halil Pasic <pasic@linux.ibm.com>
> CC: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Wow, that cleans up quite a bit of goto-spaghetti-code!

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] s390x/event-facility.c: remove unneeded labels
  2020-01-08 14:46 [PATCH v2 1/1] s390x/event-facility.c: remove unneeded labels Daniel Henrique Barboza
  2020-01-13 16:02 ` Thomas Huth
@ 2020-01-13 16:52 ` Cornelia Huck
  1 sibling, 0 replies; 3+ messages in thread
From: Cornelia Huck @ 2020-01-13 16:52 UTC (permalink / raw)
  To: Daniel Henrique Barboza
  Cc: Halil Pasic, Christian Borntraeger, Thomas Huth, qemu-devel

On Wed,  8 Jan 2020 11:46:07 -0300
Daniel Henrique Barboza <danielhb413@gmail.com> wrote:

> 'out' label from write_event_mask() and and write_event_data()

s/and and/and/

> can be replaced by 'return'.
> 
> The 'out' label from read_event_data() can also be replaced.
> However, as suggested by Cornelia Huck, instead of simply
> replacing the 'out' label, let's also change the code flow
> a bit to make it clearer that sccb events are always handled
> regardless of the mask for unconditional reads, while selective
> reads are handled if the mask is valid.
> 
> CC: Cornelia Huck <cohuck@redhat.com>
> CC: Thomas Huth <thuth@redhat.com>
> CC: Halil Pasic <pasic@linux.ibm.com>
> CC: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
> 
> This is the v2 of patch 37 sent in the series:
> 
> https://patchwork.kernel.org/cover/11319771/
> 
> after this review from Cornelia:
> 
> https://patchwork.kernel.org/patch/11319847/#23088037
> 
>  hw/s390x/event-facility.c | 33 ++++++++++++---------------------
>  1 file changed, 12 insertions(+), 21 deletions(-)

Thanks, applied.



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-01-13 16:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-08 14:46 [PATCH v2 1/1] s390x/event-facility.c: remove unneeded labels Daniel Henrique Barboza
2020-01-13 16:02 ` Thomas Huth
2020-01-13 16:52 ` Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.