[Spice-devel] [PATCH spice-server 5/8] replay: Remove time argument from recording functions

[Jonathon Jongsma]

On Fri, 2016-11-04 at 13:16 +0000, Frediano Ziglio wrote:
> Time is always the the current real time so avoid to compute
> it for every call but move to red-record-qxl.c.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red-record-qxl.c | 11 ++++++++---
>  server/red-record-qxl.h |  4 ++--
>  server/red-worker.c     | 14 +++++++-------
>  3 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
> index 8af2c9c..bd8869e 100644
> --- a/server/red-record-qxl.c
> +++ b/server/red-record-qxl.c
> @@ -803,8 +803,9 @@ void red_record_primary_surface_create(RedRecord
> *record,
>          line_0);
>  }
>  
> -void red_record_event(RedRecord *record, int what, uint32_t type,
> red_time_t ts)
> +static void red_record_event_start(RedRecord *record, int what,
> uint32_t type)
>  {
> +    red_time_t ts = spice_get_monotonic_time_ns();
>      // TODO: record the size of the packet in the header. This would
> make
>      // navigating it much faster (well, I can add an index while I'm
> at it..)
>      // and make it trivial to get a histogram from a file.
> @@ -813,12 +814,16 @@ void red_record_event(RedRecord *record, int
> what, uint32_t type, red_time_t ts)
>      fprintf(record->fd, "event %u %d %u %"PRIu64"\n", record-
> >counter++, what, type, ts);
>  }
>  
> +void red_record_event(RedRecord *record, int what, uint32_t type)
> +{
> +    red_record_event_start(record, what, type);
> +}

missing a newline here after the function definition, but...

What's the point of making this a wrapper function that simply takes
the same arguments and calls the internal function? It seems completely
unnecessary to me, and just makes things slightly confusing.



>  void red_record_qxl_command(RedRecord *record, RedMemSlotInfo
> *slots,
> -                            QXLCommandExt ext_cmd, red_time_t ts)
> +                            QXLCommandExt ext_cmd)
>  {
>      FILE *fd = record->fd;
>  
> -    red_record_event(record, 0, ext_cmd.cmd.type, ts);
> +    red_record_event_start(record, 0, ext_cmd.cmd.type);
>  
>      switch (ext_cmd.cmd.type) {
>      case QXL_CMD_DRAW:
> diff --git a/server/red-record-qxl.h b/server/red-record-qxl.h
> index 7332afe..0685393 100644
> --- a/server/red-record-qxl.h
> +++ b/server/red-record-qxl.h
> @@ -39,9 +39,9 @@ void red_record_primary_surface_create(RedRecord
> *record,
>                                         QXLDevSurfaceCreate *surface,
>                                         uint8_t *line_0);
>  
> -void red_record_event(RedRecord *record, int what, uint32_t type,
> red_time_t ts);
> +void red_record_event(RedRecord *record, int what, uint32_t type);
>  
>  void red_record_qxl_command(RedRecord *record, RedMemSlotInfo
> *slots,
> -                            QXLCommandExt ext_cmd, red_time_t ts);
> +                            QXLCommandExt ext_cmd);
>  
>  #endif
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 4383646..e1765c1 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -127,9 +127,9 @@ static int red_process_cursor(RedWorker *worker,
> int *ring_is_empty)
>              return n;
>          }
>  
> -        if (worker->record)
> -            red_record_qxl_command(worker->record, &worker-
> >mem_slots, ext_cmd,
> -                                   spice_get_monotonic_time_ns());
> +        if (worker->record) {
> +            red_record_qxl_command(worker->record, &worker-
> >mem_slots, ext_cmd);
> +        }
>  
>          worker->cursor_poll_tries = 0;
>          switch (ext_cmd.cmd.type) {
> @@ -190,9 +190,9 @@ static int red_process_display(RedWorker *worker,
> int *ring_is_empty)
>              return n;
>          }
>  
> -        if (worker->record)
> -            red_record_qxl_command(worker->record, &worker-
> >mem_slots, ext_cmd,
> -                                   spice_get_monotonic_time_ns());
> +        if (worker->record) {
> +            red_record_qxl_command(worker->record, &worker-
> >mem_slots, ext_cmd);
> +        }
>  
>          stat_inc_counter(reds, worker->command_counter, 1);
>          worker->display_poll_tries = 0;
> @@ -1033,7 +1033,7 @@ static void worker_dispatcher_record(void
> *opaque, uint32_t message_type, void *
>  {
>      RedWorker *worker = opaque;
>  
> -    red_record_event(worker->record, 1, message_type,
> spice_get_monotonic_time_ns());
> +    red_record_event(worker->record, 1, message_type);
>  }
>  
>  static void register_callbacks(Dispatcher *dispatcher)