[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive fu
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions |
Date: |
Tue, 20 Jun 2017 14:54:05 +0100 |
User-agent: |
mu4e 0.9.19; emacs 25.2.50.3 |
Peter Maydell <address@hidden> writes:
> On 19 June 2017 at 11:46, Alex Bennée <address@hidden> 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 <address@hidden>
>>
>> ---
>> 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 ?
Hmm possibly a re-base snaggle. I'll fix it up.
>
>> 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.
Yes, I'll remove
>
>> +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
--
Alex Bennée
- [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories, (continued)
- [Qemu-devel] [RISU PATCH v5 01/13] .gitignore: ignore build directories, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 02/13] README: document the coding style used for risu, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 04/13] build-all-archs: support --static flag, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 05/13] build-all-archs: support cross building via docker, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 07/13] risu: paramterise send/receive functions, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 06/13] risu: a bit more verbosity when starting, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 08/13] risu: add header to trace stream, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 10/13] risu: handle trace through stdin/stdout, Alex Bennée, 2017/06/19
- [Qemu-devel] [RISU PATCH v5 03/13] all: fix up code consitency, Alex Bennée, 2017/06/19