On 19 June 2017 at 11:46, Alex Bennée <alex.ben...@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.ben...@linaro.org>
>
> ---
> v5
> - re-base without tab/format cleanps
> v4
> - split header code
> - fix formatting foo-bar's
> v3
> - new for v3
> - arm, aarch64, ppc64
> ---
> reginfo.c | 39 +++++++++++++++++++--------------------
> risu.c | 23 +++++++++++++++++++++--
> risu.h | 11 +++++++++--
> 3 files changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/reginfo.c b/reginfo.c
> index 31bb99f..90cea8f 100644
> --- a/reginfo.c
> +++ b/reginfo.c
> @@ -21,7 +21,7 @@ uint8_t apprentice_memblock[MEMBLOCKLEN];
> static int mem_used;
> static int packet_mismatch;
>
> -int send_register_info(int sock, void *uc)
> +int send_register_info(write_fn write_fn, void *uc)
> {
> struct reginfo ri;
> int op;
> @@ -29,24 +29,25 @@ int send_register_info(int sock, void *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));
> + write_fn(&ri, sizeof(ri));
> + return 1;
Why the change from "return write function's return value"
to "ignore return value and always return 1" for OP_TESTEND ?
> case OP_COMPAREMEM:
> - return send_data_pkt(sock, memblock, MEMBLOCKLEN);
> + return write_fn(memblock, MEMBLOCKLEN);
> break;
...here we still just pass the return value through, for instance.
> + 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;
> }
> @@ -59,7 +60,7 @@ 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;
>
> @@ -73,36 +74,34 @@ int recv_and_compare_register_info(int sock, void *uc)
> /* 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))) {
> + if (read_fn(&apprentice_ri, sizeof(apprentice_ri))) {
> packet_mismatch = 1;
> resp = 2;
> -
> } else if (!reginfo_is_eq(&master_ri, &apprentice_ri)) {
> /* register mismatch */
> resp = 2;
> -
> } else if (op == OP_TESTEND) {
> resp = 1;
> }
> - send_response_byte(sock, resp);
> + resp_fn(resp);
> break;
> case OP_SETMEMBLOCK:
> - memblock = (void *) (uintptr_t) get_reginfo_paramreg(&master_ri);
> + 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);
> + (uintptr_t)memblock);
> break;
> case OP_COMPAREMEM:
> mem_used = 1;
> - if (recv_data_pkt(sock, apprentice_memblock, MEMBLOCKLEN)) {
> + if (read_fn(apprentice_memblock, MEMBLOCKLEN)) {
> packet_mismatch = 1;
> resp = 2;
> } else if (memcmp(memblock, apprentice_memblock, MEMBLOCKLEN) != 0) {
> /* memory mismatch */
> resp = 2;
> }
> - send_response_byte(sock, resp);
> + resp_fn(resp);
> break;
> }
>
> diff --git a/risu.c b/risu.c
> index a10422a..88e586c 100644
> --- a/risu.c
> +++ b/risu.c
> @@ -37,9 +37,28 @@ sigjmp_buf jmpbuf;
> /* Should we test for FP exception status bits? */
> int test_fp_exc;
>
> +/* 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 */
> advance_pc(uc);
> @@ -52,7 +71,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 */
> advance_pc(uc);
> diff --git a/risu.h b/risu.h
> index 78a7313..32241bc 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
> + */
Stale comment? The calling code is not per-arch any more.
> +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
>
thanks
-- PMM
