[Spice-devel] [PATCH] Replay: report error if we don't read the correct file header

[Jonathon Jongsma]

On Thu, 2016-01-21 at 11:48 -0500, Frediano Ziglio wrote:
> > 
> > The replay file should start with a header such as
> >   SPICE_REPLAY 1
> > 
> > Instead of soldiering on if we don't encounter this header, print a
> > warning, and also assert that spice_replay_new() returned a non-NULL
> > object.
> > ---
> >  server/red-replay-qxl.c | 3 +++
> >  server/tests/replay.c   | 1 +
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> > index 72d06c8..17ee022 100644
> > --- a/server/red-replay-qxl.c
> > +++ b/server/red-replay-qxl.c
> > @@ -1209,6 +1209,9 @@ SpiceReplay *spice_replay_new(FILE *file, int
> > nsurfaces)
> >              spice_warning("Replay file version unsupported");
> >              return NULL;
> >          }
> > +    } else {
> > +        spice_warning("This doesn't look like a valid replay file");
> > +        return NULL;
> >      }
> >  
> >      replay = spice_malloc0(sizeof(SpiceReplay));
> > diff --git a/server/tests/replay.c b/server/tests/replay.c
> > index 2af9481..2641318 100644
> > --- a/server/tests/replay.c
> > +++ b/server/tests/replay.c
> > @@ -369,6 +369,7 @@ int main(int argc, char **argv)
> >      if (total_size > 0)
> >          g_timeout_add_seconds(1, progress_timer, fd);
> >      replay = spice_replay_new(fd, MAX_SURFACE_NUM);
> > +    spice_assert(replay != NULL);
> >  
> >      aqueue = g_async_queue_new();
> >      core = basic_event_loop_init();
> 
> You are in the main and passing a wrong file is not a bug in the program, I
> would use a
> 
> if (replay != NULL) {
>    fprintf(stderr, "Error initializing replay\n");
>    exit(EXIT_FAILURE);
> }
> 
> Frediano


I figured that since it was more of a test utility, I'd just go for the quick
and dirty solution. But I agree that your suggestion is better (although you
forgot to invert the test ;)