[Spice-devel] [PATCH spice-server v4 03/20] Avoids %m in formatting for Windows

[Marc-André Lureau]

Hi

On Tue, Mar 5, 2019 at 3:02 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> > Hi
> >
> > On Wed, Feb 6, 2019 at 2:59 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> > >
> > > Not supported, %m is a GNU extension of sscanf.
> > >
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/reds.c | 25 ++++++++++++-------------
> > >  1 file changed, 12 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/server/reds.c b/server/reds.c
> > > index d3f73d8e..8c1c10dc 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -3698,8 +3698,7 @@ static const int video_codec_caps[] = {
> > >   * @codec: a location to return the parsed codec
> > >   * @return the position of the next codec in the string
> > >   */
> > > -static const char* parse_next_video_codec(const char *codecs, char
> > > **encoder,
> > > -                                          char **codec)
> > > +static char* parse_next_video_codec(char *codecs, char **encoder, char
> > > **codec)
> > >  {
> > >      if (!codecs) {
> > >          return NULL;
> > > @@ -3708,14 +3707,15 @@ static const char* parse_next_video_codec(const
> > > char *codecs, char **encoder,
> > >      if (!*codecs) {
> > >          return NULL;
> > >      }
> > > -    int n;
> > > +    int end_encoder, end_codec = -1;
> > >      *encoder = *codec = NULL;
> > > -    if (sscanf(codecs, "%m[0-9a-zA-Z_]:%m[0-9a-zA-Z_]%n", encoder, codec,
> > > &n) == 2) {
> > > -        // this avoids accepting "encoder:codec" followed by garbage like
> > > "$%*"
> > > -        if (codecs[n] != ';' && codecs[n] != '\0') {
> > > -            free(*codec);
> > > -            *codec = NULL;
> > > -        }
> > > +    if (sscanf(codecs, "%*[0-9a-zA-Z_]:%n%*[0-9a-zA-Z_];%n", &end_encoder,
> > > &end_codec) == 0
> > > +        && end_codec > 0) {
> > > +        codecs[end_encoder - 1] = '\0';
> > > +        codecs[end_codec - 1] = '\0';
> > > +        *encoder = codecs;
> > > +        *codec = codecs + end_encoder;
> > > +        return codecs + end_codec;
> > >      }
> > >      return codecs + strcspn(codecs, ";");
> > >  }
> > > @@ -3732,7 +3732,8 @@ static void
> > > reds_set_video_codecs_from_string(RedsState *reds, const char *codec
> > >      }
> > >
> > >      video_codecs = g_array_new(FALSE, FALSE, sizeof(RedVideoCodec));
> > > -    const char *c = codecs;
> > > +    char *codecs_copy = g_strdup_printf("%s;", codecs);
> > > +    char *c = codecs_copy;
> > >      while ( (c = parse_next_video_codec(c, &encoder_name, &codec_name)) )
> > >      {
> > >          uint32_t encoder_index, codec_index;
> > >          if (!encoder_name || !codec_name) {
> > > @@ -3755,11 +3756,9 @@ static void
> > > reds_set_video_codecs_from_string(RedsState *reds, const char *codec
> > >              g_array_append_val(video_codecs, new_codec);
> > >          }
> > >
> > > -        /* these are allocated by sscanf, do not use g_free */
> > > -        free(encoder_name);
> > > -        free(codec_name);
> > >          codecs = c;
> > >      }
> > > +    g_free(codecs_copy);
> > >
> >
> > codecs may refer to free memory after this, I believe.
> >
>
> No, once converted to indexes only indexes are used.

in the warning below

>
> > Would be nice to factor this parsing code out of RedsState and make
> > some unit tests.
> >
>
> Do you mean test-codecs-parsing ?

Ok I missed that one (I expected something that doesn't involve
SpiceServer, but that's fine too)

thanks

>
> >
> > >      if (video_codecs->len == 0) {
> > >          spice_warning("Failed to set video codecs, input string: '%s'",
> > >          codecs);
>
> Frediano



-- 
Marc-André Lureau