Re: [PATCH v1 4/4] hw: hyperv: Initial commit for Synthetic Debugging device

[Jon Doron <arilou@gmail.com>]

On 16/02/2022, Emanuele Giuseppe Esposito wrote:
>
>> +
>> +static uint16_t handle_recv_msg(HvSynDbg *syndbg, uint64_t outgpa,
>> +                                uint32_t count, bool is_raw, uint32_t options,
>> +                                uint64_t timeout, uint32_t *retrieved_count)
>> +{
>> +    uint16_t ret;
>> +    uint8_t data_buf[TARGET_PAGE_SIZE - UDP_PKT_HEADER_SIZE];
>> +    hwaddr out_len;
>> +    void *out_data = NULL;
>> +    ssize_t recv_byte_count;
>> +
>> +    /* TODO: Handle options and timeout */
>> +    (void)options;
>> +    (void)timeout;
>> +
>> +    if (!syndbg->has_data_pending) {
>> +        recv_byte_count = 0;
>> +    } else {
>> +        recv_byte_count = qemu_recv(syndbg->socket, data_buf,
>> +                                    MIN(sizeof(data_buf), count), MSG_WAITALL);
>> +        if (recv_byte_count == -1) {
>> +            ret = HV_STATUS_INVALID_PARAMETER;
>> +            goto cleanup;
>> +        }
>> +    }
>> +
>> +    if (!recv_byte_count) {
>> +        *retrieved_count = 0;
>> +        ret = HV_STATUS_NO_DATA;
>> +        goto cleanup;
>> +    }
>> +
>> +    set_pending_state(syndbg, false);
>> +
>> +    out_len = recv_byte_count;
>> +    if (is_raw) {
>> +        out_len += UDP_PKT_HEADER_SIZE;
>> +    }
>> +    out_data = cpu_physical_memory_map(outgpa, &out_len, 1);
>> +    if (!out_data) {
>> +        ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> +        goto cleanup;
>> +    }
>> +
>> +    if (is_raw &&
>> +        !create_udp_pkt(syndbg, out_data,
>> +                        recv_byte_count + UDP_PKT_HEADER_SIZE,
>> +                        data_buf, recv_byte_count)) {
>> +        ret = HV_STATUS_INSUFFICIENT_MEMORY;
>> +        goto cleanup;
>> +    } else if (!is_raw) {
>> +        memcpy(out_data, data_buf, recv_byte_count);
>> +    }
>> +
>> +    *retrieved_count = recv_byte_count;
>> +    if (is_raw) {
>> +        *retrieved_count += UDP_PKT_HEADER_SIZE;
>> +    }
>> +    ret = HV_STATUS_SUCCESS;
>> +cleanup:
>> +    if (out_data) {
>> +        cpu_physical_memory_unmap(out_data, out_len, 1, out_len);
>> +    }
>
>Same nitpick as done in patch 1, I think you can use more gotos labels
>instead of adding if statements.
>
Done
>> +
>> +    return ret;
>> +}
>> +
>