This is a precursor to record/playback support. Instead of passing the
socket fd we now pass helper functions for reading/writing and
responding. This will allow us to do the rest of the record/playback
cleanly outside of the main worker function.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v4
  - split header code
  - fix formatting foo-bar's
v3
  - new for v3
  - arm, aarch64, ppc64
---
 reginfo.c | 118 +++++++++++++++++++++++++++++++-------------------------------
 risu.c    |  23 ++++++++++--
 risu.h    |  11 ++++--
 3 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/reginfo.c b/reginfo.c
index 96c6342..6498459 100644
--- a/reginfo.c
+++ b/reginfo.c
@@ -21,33 +21,35 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
 static int mem_used = 0;
 static int packet_mismatch = 0;
 
-int send_register_info(int sock, void *uc)
+int send_register_info(write_fn write_fn, void *uc)
 {
-    struct reginfo ri;
-    int op;
-    reginfo_init(&ri, uc);
-    op = get_risuop(&ri);
-
-    switch (op) {
-    case OP_COMPARE:
-    case OP_TESTEND:
-    default:
-        /* Do a simple register compare on (a) explicit request
-         * (b) end of test (c) a non-risuop UNDEF
-         */
-        return send_data_pkt(sock, &ri, sizeof(ri));
-    case OP_SETMEMBLOCK:
-        memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
-       break;
-    case OP_GETMEMBLOCK:
-        set_ucontext_paramreg(uc,
-                              get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
-        break;
-    case OP_COMPAREMEM:
-        return send_data_pkt(sock, memblock, MEMBLOCKLEN);
-        break;
-    }
-    return 0;
+   struct reginfo ri;
+   int op;
+   reginfo_init(&ri, uc);
+   op = get_risuop(&ri);
+
+   switch (op) {
+      case OP_TESTEND:
+         write_fn(&ri, sizeof(ri));
+         return 1;
+      case OP_SETMEMBLOCK:
+         memblock = (void *)(uintptr_t)get_reginfo_paramreg(&ri);
+         break;
+      case OP_GETMEMBLOCK:
+         set_ucontext_paramreg(uc,
+                               get_reginfo_paramreg(&ri) + (uintptr_t)memblock);
+         break;
+      case OP_COMPAREMEM:
+         return write_fn(memblock, MEMBLOCKLEN);
+         break;
+      case OP_COMPARE:
+      default:
+         /* Do a simple register compare on (a) explicit request
+          * (b) end of test (c) a non-risuop UNDEF
+          */
+         return write_fn(&ri, sizeof(ri));
+   }
+   return 0;
 }
 
 /* Read register info from the socket and compare it with that from the
@@ -58,54 +60,52 @@ int send_register_info(int sock, void *uc)
  * that says whether it is register or memory data, so if the two
  * sides get out of sync then we will fail obscurely.
  */
-int recv_and_compare_register_info(int sock, void *uc)
+int recv_and_compare_register_info(read_fn read_fn, respond_fn resp_fn, void *uc)
 {
-    int resp = 0, op;
-
-    reginfo_init(&master_ri, uc);
-    op = get_risuop(&master_ri);
-
-    switch (op) {
-    case OP_COMPARE:
-    case OP_TESTEND:
-    default:
-        /* Do a simple register compare on (a) explicit request
-         * (b) end of test (c) a non-risuop UNDEF
-         */
-        if (recv_data_pkt(sock, &apprentice_ri, sizeof(apprentice_ri))) {
+   int resp = 0, op;
+
+   reginfo_init(&master_ri, uc);
+   op = get_risuop(&master_ri);
+
+   switch (op) {
+      case OP_COMPARE:
+      case OP_TESTEND:
+      default:
+         /* Do a simple register compare on (a) explicit request
+          * (b) end of test (c) a non-risuop UNDEF
+          */
+         if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
             packet_mismatch = 1;
             resp = 2;
-
-        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
+         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
             /* register mismatch */
             resp = 2;
-
-        } else if (op == OP_TESTEND) {
+         } else if (op == OP_TESTEND) {
             resp = 1;
-        }
-        send_response_byte(sock, resp);
-        break;
+         }
+         resp_fn(resp);
+         break;
       case OP_SETMEMBLOCK:
-          memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
-          break;
+         memblock = (void *)(uintptr_t)get_reginfo_paramreg(&master_ri);
+         break;
       case OP_GETMEMBLOCK:
-          set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
-                                (uintptr_t)memblock);
-          break;
+         set_ucontext_paramreg(uc, get_reginfo_paramreg(&master_ri) +
+                               (uintptr_t)memblock);
+         break;
       case OP_COMPAREMEM:
          mem_used = 1;
-         if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
-             packet_mismatch = 1;
-             resp = 2;
+         if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
+            packet_mismatch = 1;
+            resp = 2;
          } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
-             /* memory mismatch */
-             resp = 2;
+            /* memory mismatch */
+            resp = 2;
          }
-         send_response_byte(sock, resp);
+         resp_fn(resp);
          break;
    }
 
-    return resp;
+   return resp;
 }
 
 /* Print a useful report on the status of the last comparison
diff --git a/risu.c b/risu.c
index bcdc219..22571cd 100644
--- a/risu.c
+++ b/risu.c
@@ -47,9 +47,28 @@ void report_test_status(void *pc)
    }
 }
 
+/* Master functions */
+
+int read_sock(void *ptr, size_t bytes)
+{
+   return recv_data_pkt(master_socket, ptr, bytes);
+}
+
+void respond_sock(int r)
+{
+   send_response_byte(master_socket, r);
+}
+
+/* Apprentice function */
+
+int write_sock(void *ptr, size_t bytes)
+{
+   return send_data_pkt(apprentice_socket, ptr, bytes);
+}
+
 void master_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (recv_and_compare_register_info(master_socket, uc))
+   switch (recv_and_compare_register_info(read_sock, respond_sock, uc))
    {
       case 0:
          /* match OK */
@@ -63,7 +82,7 @@ void master_sigill(int sig, siginfo_t *si, void *uc)
 
 void apprentice_sigill(int sig, siginfo_t *si, void *uc)
 {
-   switch (send_register_info(apprentice_socket, uc))
+   switch (send_register_info(write_sock, uc))
    {
       case 0:
          /* match OK */
diff --git a/risu.h b/risu.h
index 1eeb885..71ea94f 100644
--- a/risu.h
+++ b/risu.h
@@ -53,17 +53,24 @@ struct reginfo;
 
 /* Functions operating on reginfo */
 
+/* To keep the read/write logic from multiplying across all arches
+ * we wrap up the function here to keep all the changes in one place
+ */
+typedef int (*write_fn) (void *ptr, size_t bytes);
+typedef int (*read_fn) (void *ptr, size_t bytes);
+typedef void (*respond_fn) (int response);
+
 /* Send the register information from the struct ucontext down the socket.
  * Return the response code from the master.
  * NB: called from a signal handler.
  */
-int send_register_info(int sock, void *uc);
+int send_register_info(write_fn write_fn, void *uc);
 
 /* Read register info from the socket and compare it with that from the
  * ucontext. Return 0 for match, 1 for end-of-test, 2 for mismatch.
  * NB: called from a signal handler.
  */
-int recv_and_compare_register_info(int sock, void *uc);
+int recv_and_compare_register_info(read_fn read_fn, respond_fn respond, void *uc);
 
 /* Print a useful report on the status of the last comparison
  * done in recv_and_compare_register_info(). This is called on
-- 
2.13.0

On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
> This is a precursor to record/playback support. Instead of passing the
> socket fd we now pass helper functions for reading/writing and
> responding. This will allow us to do the rest of the record/playback
> cleanly outside of the main worker function.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> -        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
> +         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
>              /* register mismatch */
>              resp = 2;
> -
> -        } else if (op == OP_TESTEND) {
> +         } else if (op == OP_TESTEND) {
>              resp = 1;
> -        }

[etc]

If you're going to fix the indent can you put that in
its own patch, please? Much easier to review that way.

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On 2 June 2017 at 17:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This is a precursor to record/playback support. Instead of passing the
>> socket fd we now pass helper functions for reading/writing and
>> responding. This will allow us to do the rest of the record/playback
>> cleanly outside of the main worker function.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
>> -        } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
>> +         } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
>>              /* register mismatch */
>>              resp = 2;
>> -
>> -        } else if (op == OP_TESTEND) {
>> +         } else if (op == OP_TESTEND) {
>>              resp = 1;
>> -        }
>
> [etc]
>
> If you're going to fix the indent can you put that in
> its own patch, please? Much easier to review that way.

Damn I forgotten that got munged. Can we agree on what the indent should
be for risu? 3 or 4, space or tabs?

--
Alex Bennée

On 6 June 2017 at 11:13, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> If you're going to fix the indent can you put that in
>> its own patch, please? Much easier to review that way.
>
> Damn I forgotten that got munged. Can we agree on what the indent should
> be for risu? 3 or 4, space or tabs?

Same as QEMU coding style, please.

thanks
-- PMM