Hi
On Wed, Feb 6, 2019 at 2:59 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> Not supported, %m is a GNU extension of sscanf.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> 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.
Would be nice to factor this parsing code out of RedsState and make
some unit tests.
> if (video_codecs->len == 0) {
> spice_warning("Failed to set video codecs, input string: '%s'", codecs);
> --
> 2.20.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel